Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds a comprehensive test framework for validating dynamic population functionality in Tethys. The test compares model outputs between static population (using only 2010 data) and dynamic population (using both 2010 and 2020 data with interpolation) scenarios to verify that temporal interpolation of proxy data is working correctly.
Key Changes:
- Added
DynamicPopulationTestclass that creates and runs two scenarios with different population proxy configurations - Implemented validation checks to verify baseline year equality and interpolation behavior across multiple years (2010-2030)
- Added comprehensive reporting functionality with statistical comparisons and pass/fail validation
Comments suppressed due to low confidence (1)
tests/test_dynamic_population.py:1
- Catching broad
Exceptioncan hide programming errors and make debugging harder. Consider catching specific expected exceptions (e.g.,FileNotFoundError,ValueError,yaml.YAMLError) and letting unexpected exceptions propagate to reveal genuine bugs during testing.
"""
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| dynamic_da = self.dynamic_results[sector] | ||
|
|
||
| # Ensure same coordinates | ||
| static_da = static_da.reindex_like(dynamic_da, fill_value=0) |
There was a problem hiding this comment.
Using fill_value=0 for reindexing could mask data alignment issues. If the static and dynamic results have different coordinates, filling with 0 will silently create artificial zeros rather than highlighting a potential problem. Consider using fill_value=np.nan to make missing data explicit, or validating that coordinates match before reindexing.
| static_da = static_da.reindex_like(dynamic_da, fill_value=0) | |
| static_da = static_da.reindex_like(dynamic_da, fill_value=np.nan) |
| test._load_base_config() | ||
|
|
||
| # Create config files | ||
| test._create_static_config() | ||
| test._create_dynamic_config() |
There was a problem hiding this comment.
Calling private methods (prefixed with _) from outside the class violates encapsulation. Consider creating a public method like setup() or prepare_scenarios() that orchestrates these private method calls, or make these methods public if they're intended to be called externally.
|
|
||
| import os | ||
| import yaml | ||
| import xarray as xr |
There was a problem hiding this comment.
Import of 'xr' is not used.
| import xarray as xr |
| import pandas as pd | ||
| from pathlib import Path | ||
|
|
||
| import tethys |
There was a problem hiding this comment.
Import of 'tethys' is not used.
| import tethys |
|
I ran the tests manually and everything passed, from what I can tell this has already been used in the new runs so it should be good to merge |
Goal: Test to ensure population data can be passed in dynamically to represent and influence future years in the model against the preexisting use of static population data.
To run the test script and produce a report in the terminal:
python tests/test_dynamic_population.pywhere thetethyspackage has been installed in a virtual environment that is active in the terminal where the above command is ran.This also needs further evaluation from a domain scientist to ensure that the outcomes between the use of static and dynamic population are as expected.