Conversation
- Fix version mismatch: __init__.py now matches pyproject.toml (1.2.3) - Fix FIA.__exit__ no-op: added close() to properly disconnect DB backend - Remove pandas dependency: not used anywhere in src/pyfia/ - Fix hardcoded year 2023 default in format_output - Preserve AREA_TOTAL column in aggregation output (was dropped prematurely) - Replace lru_cache on instance method with manual cache to prevent GC leak - Migrate carbon_pools to AggregationResult pattern, remove dead code - Add algebraic identity documentation to variance formula - Use NotImplementedError for aggregate_results (not abstract) to support estimators like AreaChangeEstimator that override estimate() entirely All 677 unit tests pass.
…mples Bug fixes: - Fix Issue #68: Area variance severely underestimated for rare categories when using grp_by. Now cross-joins all stratum plots with group values so non-matching plots contribute y=0 to variance calculation. - Fix LazyFrame/DataFrame type mismatch in batch_query_by_values concat - Fix EVALIDator client using incorrect JSONDecodeError for empty responses - Pass area_domain to get_cond_columns in all estimators so domain filter columns are loaded from the database - Update area_change calculate_variance signature for AggregationResult Cleanup: - Remove deprecated assign_forest_type_group (use add_forest_type_group) - Remove 5 outdated example scripts, modernize 3 remaining ones - Add descriptive aliases for EVALIDator estimate type codes - Fix data_reader type annotation for MotherDuck path Tests: - Add 4 tests for rare category variance (Issue #68) - Update classification tests to use add_forest_type_group directly - Update reference table download test for bundled zip handling - Improve biomass validation skip reason documentation
The base class was fixed to use _extract_evaluation_year() but the area estimator overrides format_output and still had pl.lit(2023). Now uses the same pattern as all other estimators.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Commit 1: Critical review bug fixes and technical debt
__init__.pywas1.1.0b7, now matchespyproject.toml(1.2.3)FIA.__exit__no-op: Addedclose()method to properly disconnect DB backend on context manager exitsrc/pyfia/format_outputnow uses_extract_evaluation_year()instead ofconfig.get("year", 2023)AREA_TOTALcolumn: Was dropped incompute_per_acre_valuesdespite being documented as outputlru_cachememory leak: Replaced@lru_cacheon instance methodget_stratification_datawith manual instance-level cachecarbon_poolstoAggregationResultpattern: Eliminated last estimator using old instance-state pattern; removed dead codeW_h²×n_hand textbookN_h²/n_hformsCommit 2: Area variance fix, deprecated code removal, example updates
grp_by. Now cross-joins all stratum plots with group values so non-matching plots contribute y=0batch_query_by_valuesJSONDecodeErrorfor empty responsesarea_domaintoget_cond_columnsin all estimators so domain filter columns are loadedassign_forest_type_group(useadd_forest_type_group)Test plan
FIAcontext manager properly disconnects withclose()