Weather morpher #3916
Weather morpher #3916
Conversation
Introduced new weather morphing parameters in default.config, including year, climate pathway, percentile, and variables. Also added pyepwmorph import in weather_helper.py to support weather file morphing functionality.
Integrating pyepwmorph into CEA as a part of the Weather Helper. Have introduced general functionality. Need to make a new pyepwmorph version and then another scenario will become available.
WalkthroughAdds EPW climate morphing: new epwmorpher module using pyepwmorph, a morph toggle in weather_helper, morph-related config options, a utility to read EPW location metadata, and a pyepwmorph runtime dependency. The morph workflow validates, backs up, compiles model data, performs morphing, and writes a morphed EPW. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Config as Config
participant WH as weather_helper
participant EPM as epwmorpher
participant PIO as pyepwmorph_io.Epw
participant PWF as pyepwmorph_workflow
participant FS as Filesystem
User->>WH: Run weather helper
WH->>Config: Read weather-helper.morph
alt morph == true
WH->>EPM: main(config)
EPM->>FS: Copy original -> before_morph_weather.epw
EPM->>PIO: Load and validate EPW
EPM->>PIO: Detect baseline range (or warn/fallback)
EPM->>Config: Read year, pathway, percentile, variables
EPM->>PWF: iterate_compile_model_data(...)
EPM->>PWF: morph_epw(...)
PWF-->>EPM: Morphed EPW
EPM->>FS: Write output `weather.epw`
else morph == false
WH->>FS: Validate weather file presence
alt Source is online
WH->>External: Fetch from Climate.OneBuilding.org
External-->>WH: EPW file
WH->>FS: Save EPW
else Local file
WH->>FS: Copy provided EPW
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
cea/datamanagement/weather_helper/epwmorpher.py (1)
93-96: Consider making model sources configurable.The hard-coded model sources list might benefit from being configurable, especially as model availability changes over time.
Consider extracting the model sources to a configuration parameter or constant:
# At module level DEFAULT_MODEL_SOURCES = [ 'KACE-1-0-G', 'MRI-ESM2-0', 'GFDL-ESM4', 'INM-CM4-8', 'IPSL-CM6A-LR', 'INM-CM5-0', 'ACCESS-CM2', 'MIROC6', 'EC-Earth3-Veg-LR', 'BCC-CSM2-MR' ] # In the function model_sources = getattr(config.weather_helper, 'model_sources', DEFAULT_MODEL_SOURCES)pyproject.toml (1)
68-68: Minor: pyepwmorph dependency — package present, no advisories; consider loosening the upper bound.PyPI reports pyepwmorph latest 0.4.12 and the GitHub security query returned no vulnerabilities; current constraint (>=0.3.1,<0.4) excludes 0.4.x — widen or remove the upper bound if 0.4.x is compatible so you can receive fixes/patches.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pixi.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
cea/datamanagement/weather_helper/epwmorpher.py(1 hunks)cea/datamanagement/weather_helper/weather_helper.py(2 hunks)cea/default.config(1 hunks)cea/utilities/epwreader.py(1 hunks)pyproject.toml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-12-10T14:11:58.324Z
Learnt from: reyery
PR: architecture-building-systems/CityEnergyAnalyst#3713
File: cea/datamanagement/weather_helper/weather_helper.py:94-94
Timestamp: 2024-12-10T14:11:58.324Z
Learning: When converting .dat weather files in `parse_dat_weather_file`, extract location information from the first 5 lines of the `.dat` file header instead of using hardcoded values.
Applied to files:
cea/utilities/epwreader.py
🧬 Code graph analysis (2)
cea/datamanagement/weather_helper/epwmorpher.py (3)
cea/datamanagement/weather_helper/weather_helper.py (1)
main(80-105)cea/inputlocator.py (1)
InputLocator(20-1505)cea/config.py (1)
Configuration(35-277)
cea/datamanagement/weather_helper/weather_helper.py (2)
cea/datamanagement/weather_helper/epwmorpher.py (1)
main(139-147)cea/inputlocator.py (2)
ensure_parent_folder_exists(104-106)get_zone_geometry(883-887)
🪛 GitHub Actions: Ruff
cea/datamanagement/weather_helper/epwmorpher.py
[error] 11-11: F401: 'datetime' imported but unused. Remove unused import.
[error] 74-74: E722: Do not use bare 'except'.
[error] 1-1: Ruff check failed: 2 errors found; run 'ruff --fix' to address issues.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: tests (macos-latest)
- GitHub Check: tests (ubuntu-latest)
- GitHub Check: tests (windows-latest)
🔇 Additional comments (14)
cea/utilities/epwreader.py (1)
75-90: LGTM! Simple and effective EPW location parser.The function correctly extracts location metadata from the EPW header line using standard CSV parsing. The implementation is straightforward and handles the required fields properly.
cea/datamanagement/weather_helper/weather_helper.py (4)
13-13: LGTM! Clean integration of the morphing module.The import statement properly adds the epwmorpher module for the new morphing functionality.
90-93: Good integration of morphing workflow with clear user feedback.The conditional logic cleanly separates morphing from regular weather handling, and the informative message helps users understand what happened during morphing.
95-99: Improved error handling with clear guidance.The enhanced error message provides users with actionable guidance when no weather file is provided.
100-105: LGTM! Proper file handling and control flow.The code ensures the parent directory exists and maintains the existing logic for both online fetching and file copying scenarios.
cea/default.config (4)
128-131: LGTM! Well-designed morphing toggle parameter.The boolean parameter with clear help text provides an intuitive way to enable/disable weather morphing.
132-135: LGTM! Appropriate default year for climate projections.Setting 2050 as the default year aligns well with typical climate planning horizons.
143-147: LGTM! Good choice of median percentile as default.Using the 50th percentile (median) as the default provides a balanced middle-ground approach for climate projections.
149-152: LGTM! Temperature as default variable makes sense.Starting with temperature as the default morphing variable is logical since it's often the primary climate concern.
cea/datamanagement/weather_helper/epwmorpher.py (5)
25-43: LGTM! Clear scenario name mapping with good error handling.The function provides explicit mappings between CEA and pyepwmorph scenario names and includes a helpful error message for unsupported values.
45-63: LGTM! Good file handling and validation.The function properly constructs the project name, handles file copying for backup, and includes appropriate file existence checks.
99-107: LGTM! Well-structured configuration object creation.The MorphConfig initialization uses clear parameter mapping and proper conversion of scenario names.
113-118: LGTM! Proper workflow integration with pyepwmorph.The climate model data retrieval correctly uses the configuration parameters and follows the pyepwmorph workflow.
123-135: LGTM! Clean morphing and output handling.The morphing process is well-structured, properly updates the year field, and writes the output to the expected location.
| try: | ||
| baseline_range = user_epw_object.detect_baseline_range() | ||
| except: | ||
| print("Could not detect the baseline range from the EPW file, using default of 1985-2014") | ||
| baseline_range = (1985, 2014) # default if the EPW file does not have the years in it |
There was a problem hiding this comment.
Replace bare except clause with specific exception handling.
Using a bare except: clause can mask unexpected errors and make debugging difficult.
Apply this diff to use more specific exception handling:
try:
baseline_range = user_epw_object.detect_baseline_range()
-except:
+except Exception as e:
print("Could not detect the baseline range from the EPW file, using default of 1985-2014")
+ print(f"Error details: {e}")
baseline_range = (1985, 2014) # default if the EPW file does not have the years in it📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try: | |
| baseline_range = user_epw_object.detect_baseline_range() | |
| except: | |
| print("Could not detect the baseline range from the EPW file, using default of 1985-2014") | |
| baseline_range = (1985, 2014) # default if the EPW file does not have the years in it | |
| try: | |
| baseline_range = user_epw_object.detect_baseline_range() | |
| except Exception as e: | |
| print("Could not detect the baseline range from the EPW file, using default of 1985-2014") | |
| print(f"Error details: {e}") | |
| baseline_range = (1985, 2014) # default if the EPW file does not have the years in it |
🧰 Tools
🪛 GitHub Actions: Ruff
[error] 74-74: E722: Do not use bare 'except'.
🤖 Prompt for AI Agents
In cea/datamanagement/weather_helper/epwmorpher.py around lines 72 to 76, the
try/except uses a bare except which can hide unexpected errors; replace it with
specific exception handling (for example catch AttributeError and ValueError
that detect_baseline_range might raise), and if you must keep a generic fallback
catch use "except Exception as e" and log the exception (via logging or the
module's logger) before assigning the default baseline_range = (1985, 2014).
| def main(config): | ||
| """ | ||
| This script gets a polygon and calculates the zone.shp and the occupancy.dbf and age.dbf inputs files for CEA | ||
| """ | ||
| assert os.path.exists( | ||
| config.scenario), 'Scenario not found: %s' % config.scenario | ||
| locator = cea.inputlocator.InputLocator(config.scenario) | ||
| print(f"{'=' * 10} Starting the climate morphing workflow for scenario {config.general.scenario} {'=' * 10}") | ||
| morphing_workflow(locator, config) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update docstring to match actual functionality.
The docstring appears to be copied from another function and doesn't describe what this function actually does.
Apply this diff to fix the docstring:
def main(config):
"""
- This script gets a polygon and calculates the zone.shp and the occupancy.dbf and age.dbf inputs files for CEA
+ Main entry point for EPW weather file morphing using pyepwmorph.
+
+ :param cea.config.Configuration config: Configuration object containing morphing parameters
"""📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def main(config): | |
| """ | |
| This script gets a polygon and calculates the zone.shp and the occupancy.dbf and age.dbf inputs files for CEA | |
| """ | |
| assert os.path.exists( | |
| config.scenario), 'Scenario not found: %s' % config.scenario | |
| locator = cea.inputlocator.InputLocator(config.scenario) | |
| print(f"{'=' * 10} Starting the climate morphing workflow for scenario {config.general.scenario} {'=' * 10}") | |
| morphing_workflow(locator, config) | |
| def main(config): | |
| """ | |
| Main entry point for EPW weather file morphing using pyepwmorph. | |
| :param cea.config.Configuration config: Configuration object containing morphing parameters | |
| """ | |
| assert os.path.exists( | |
| config.scenario), 'Scenario not found: %s' % config.scenario | |
| locator = cea.inputlocator.InputLocator(config.scenario) | |
| print(f"{'=' * 10} Starting the climate morphing workflow for scenario {config.general.scenario} {'=' * 10}") | |
| morphing_workflow(locator, config) |
🤖 Prompt for AI Agents
In cea/datamanagement/weather_helper/epwmorpher.py around lines 139 to 147, the
function main has an incorrect copied docstring; replace it with a short
accurate docstring stating that main runs the climate morphing workflow for a
given CEA config: it validates the scenario path, creates an InputLocator,
prints a start message and calls morphing_workflow(locator, config). Keep it
concise and reflect the actual parameters and behavior.
| climate-pathway = moderate_case | ||
| climate-pathway.type = ChoiceParameter | ||
| climate-pathway.choices = Best Case, Moderate, Worst Case | ||
| climate-pathway.help = Climate pathway to morph the weather file to. The "Best Case" pathway corresponds to a 1.8°C increase in global temperature, the "Moderate" pathway corresponds to a 2.7°C increase, and the "Worst Case" pathway corresponds to a 4.4°C increase. More information about these pathways can be read here https://www.carbonbrief.org/explainer-how-shared-socioeconomic-pathways-explore-future-climate-change/. | ||
| climate-pathway.category = Morph Settings |
There was a problem hiding this comment.
Consider correcting the default value format.
The default value should match one of the choices. The choices use spaces, but the default uses underscores.
Apply this diff to fix the default value:
-climate-pathway = moderate_case
+climate-pathway = Moderate📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| climate-pathway = moderate_case | |
| climate-pathway.type = ChoiceParameter | |
| climate-pathway.choices = Best Case, Moderate, Worst Case | |
| climate-pathway.help = Climate pathway to morph the weather file to. The "Best Case" pathway corresponds to a 1.8°C increase in global temperature, the "Moderate" pathway corresponds to a 2.7°C increase, and the "Worst Case" pathway corresponds to a 4.4°C increase. More information about these pathways can be read here https://www.carbonbrief.org/explainer-how-shared-socioeconomic-pathways-explore-future-climate-change/. | |
| climate-pathway.category = Morph Settings | |
| climate-pathway = Moderate | |
| climate-pathway.type = ChoiceParameter | |
| climate-pathway.choices = Best Case, Moderate, Worst Case | |
| climate-pathway.help = Climate pathway to morph the weather file to. The "Best Case" pathway corresponds to a 1.8°C increase in global temperature, the "Moderate" pathway corresponds to a 2.7°C increase, and the "Worst Case" pathway corresponds to a 4.4°C increase. More information about these pathways can be read here https://www.carbonbrief.org/explainer-how-shared-socioeconomic-pathways-explore-future-climate-change/. | |
| climate-pathway.category = Morph Settings |
🤖 Prompt for AI Agents
In cea/default.config around lines 137 to 141, the default value for
climate-pathway ("moderate_case") does not match any of the declared choices
(Best Case, Moderate, Worst Case); update the default to exactly one of the
choices — replace climate-pathway = moderate_case with climate-pathway =
Moderate so the default matches the choices list and preserves casing and
spacing.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
cea/datamanagement/weather_helper/epwmorpher.py (1)
146-149: Clean up commented code.Remove the commented-out code block to maintain code cleanliness.
- # morphed_data.write_to_file(os.path.join(morph_config.output_directory, - # f"{str(user_future_year)}_{user_climate_pathway}_{percentile_key}.epw")) -
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cea/datamanagement/weather_helper/epwmorpher.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
cea/datamanagement/weather_helper/epwmorpher.py (2)
cea/datamanagement/weather_helper/weather_helper.py (1)
main(80-105)cea/inputlocator.py (1)
InputLocator(20-1505)
🪛 GitHub Actions: Ruff
cea/datamanagement/weather_helper/epwmorpher.py
[error] 10-10: F401: 'datetime' imported but unused. Remove unused import. (ruff)
[error] 88-88: E722: Do not use bare except. Use a specific exception type. (ruff)
🔇 Additional comments (8)
cea/datamanagement/weather_helper/epwmorpher.py (8)
10-10: Remove unused datetime import.The
datetimemodule is imported but never used in this file, as confirmed by the pipeline failure.
32-50: LGTM! Clear scenario name mapping with proper error handling.The function correctly maps CEA scenario names to pyepwmorph equivalents and raises a descriptive ValueError for unsupported inputs.
61-77: LGTM! Proper project setup and file validation.The code correctly constructs the project name, creates a backup of the original EPW file, ensures the output directory exists, and validates the EPW file existence with appropriate error handling.
86-90: Replace bare except with specific exception handling.Using a bare
except:clause can mask unexpected errors and make debugging difficult, as flagged by the pipeline failure.
107-110: Verify model sources are current and complete.The hardcoded list of climate model sources includes a comment indicating they're from 2025.09.23. Ensure these models are still valid and complete for the intended use cases.
Based on the search results, I can see that CMIP6 models are actively used and documented, but I need more specific information about the exact model names listed in the code to verify their validity.Perfect! The search results confirm that the model names listed in the code are valid CMIP6 models. I can see multiple references to these models:
- KACE-1-0-G is confirmed as a valid CMIP6 model
- MRI-ESM2-0 is confirmed as a valid CMIP6 model
- GFDL-ESM4 is confirmed as a valid CMIP6 model
- INM-CM4-8 and INM-CM5-0 are confirmed as valid CMIP6 models
- IPSL-CM6A-LR, ACCESS-CM2, MIROC6, EC-Earth3-Veg-LR, BCC-CSM2-MR are all confirmed as valid CMIP6 models
All the models listed in the code are legitimate CMIP6 models used in climate research and downscaling applications.
113-121: LGTM! Well-structured morph configuration.The MorphConfig object is properly instantiated with all necessary parameters, including proper list wrapping for scenario names and percentiles as expected by the pyepwmorph library.
127-143: LGTM! Proper climate data compilation and morphing workflow.The code correctly:
- Compiles climate model data using geographic coordinates from the EPW file
- Applies morphing with appropriate parameters
- Updates the year in the morphed data to match the target year
153-164: Update docstring to reflect actual functionality.The docstring appears to be copied from another function and doesn't accurately describe this function's purpose.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
cea/datamanagement/weather_helper/epwmorpher.py (1)
81-82: Normalize variables to list to avoid downstream type errors.If a single variable is provided as a string in config, pyepwmorph may expect a list. Safe-cast here.
Apply this diff:
- user_variables = config.weather_helper.variables + user_variables = config.weather_helper.variables + if isinstance(user_variables, str): + user_variables = [user_variables]
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cea/datamanagement/weather_helper/epwmorpher.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
cea/datamanagement/weather_helper/epwmorpher.py (2)
cea/datamanagement/weather_helper/weather_helper.py (1)
main(80-105)cea/inputlocator.py (1)
InputLocator(20-1505)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: tests (ubuntu-latest)
- GitHub Check: tests (macos-latest)
- GitHub Check: tests (windows-latest)
🔇 Additional comments (3)
cea/datamanagement/weather_helper/epwmorpher.py (3)
85-90: Capture exception details (and prefer logging over print).You switched away from a bare except (good), but you’re discarding the exception details. At minimum, include the exception in the message; ideally, use a logger.
Minimal change:
- except Exception: - print("Could not detect the baseline range from the EPW file, using default of 1985-2014") + except Exception as e: + print(f"Could not detect the baseline range from the EPW file, using default of 1985-2014. Details: {e}")Recommended (logging):
+import logging +logger = logging.getLogger(__name__) @@ - except Exception as e: - print(f"Could not detect the baseline range from the EPW file, using default of 1985-2014. Details: {e}") + except Exception as e: + logger.warning( + "Could not detect the baseline range from the EPW file, defaulting to 1985–2014. Details: %s", e + )
152-158: Docstring is accurate and clear.Entry-point docs now reflect the actual morphing workflow and parameters.
62-64: config.general.scenario is defined
Tests in cea/tests/test_config.py confirm config.general.scenario always exists and equals config.scenario; no AttributeError risk and no change needed.Likely an incorrect or invalid review comment.
| # pyepwmorph import | ||
| from pyepwmorph.tools import workflow as pyepwmorph_workflow | ||
| from pyepwmorph.tools import utilities as pyepwmorph_utilities | ||
| from pyepwmorph.tools import io as pyepwmorph_io | ||
| from pyepwmorph.tools import configuration as pyepwmorph_config | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
Confirm pyepwmorph version compatibility and pin appropriately.
pyepwmorph ≥0.4.12 contains relevant bug fixes (e.g., DNI > 1000 handling) but requires Python ≥3.10. If CEA’s runtime meets this, consider pinning to 0.4.12 for stability; otherwise ensure current pin documents rationale.
🌐 Web query:
What is the latest version of pyepwmorph and its Python version requirement?
💡 Result:
Latest pyepwmorph version: 0.4.12 (released Oct 24, 2024). Python requirement: Python >= 3.10. [1][2]
Sources:
[1] PyPI — pyepwmorph 0.4.12 (project page).
[2] Wheelodex / package metadata for pyepwmorph 0.4.12.
🏁 Script executed:
#!/bin/bash
set -e
echo "=== python_requires / requires-python checks ==="
rg -n 'python_requires' -g '*.{py,toml,cfg,ini}' || echo "No python_requires found"
rg -n 'requires-python' -g 'pyproject.toml' || echo "No requires-python in pyproject.toml"
echo
echo "=== GitHub Actions Python versions ==="
rg -n 'python-version' -g '.github/workflows/*.yml' || echo "No python-version entries in workflows"
echo
echo "=== Dependency pin checks for pyepwmorph ==="
rg -n 'pyepwmorph' -g 'requirements*' -C3 || echo "No explicit pyepwmorph pin in requirements files"
rg -n 'pyepwmorph' -g 'Pipfile' -C3 || echo "No explicit pyepwmorph pin in Pipfile"Length of output: 834
Pin pyepwmorph to ≥0.4.12
Add pyepwmorph>=0.4.12,<0.5.0 to your dependency spec (pyproject.toml or requirements.txt); project already targets Python ≥3.10.
🤖 Prompt for AI Agents
In cea/datamanagement/weather_helper/epwmorpher.py around lines 16 to 21, the
project currently imports pyepwmorph but does not enforce a minimum compatible
version; update your dependency spec to pin pyepwmorph to a safe range by adding
"pyepwmorph>=0.4.12,<0.5.0" to your dependency file (pyproject.toml or
requirements.txt), then run dependency install/update (poetry install or pip
install -r requirements.txt) and update any lockfile so the project uses the
specified version range.
| raise ValueError(f"Could not interpret the climate pathway: {name}. " | ||
| f"Please choose one of the following: best_case, moderate_case, semi_moderate_case, worst_case") |
There was a problem hiding this comment.
Align error message with actual accepted inputs.
The message lists snake_case names and “semi_moderate_case”, which don’t match the accepted inputs here.
Apply this diff:
- raise ValueError(f"Could not interpret the climate pathway: {name}. "
- f"Please choose one of the following: best_case, moderate_case, semi_moderate_case, worst_case")
+ raise ValueError(
+ f"Unsupported climate pathway: {name!r}. "
+ "Please choose one of: 'Best Case', 'Moderate', 'Upper Middle', or 'Worst Case'."
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| raise ValueError(f"Could not interpret the climate pathway: {name}. " | |
| f"Please choose one of the following: best_case, moderate_case, semi_moderate_case, worst_case") | |
| raise ValueError( | |
| f"Unsupported climate pathway: {name!r}. " | |
| "Please choose one of: 'Best Case', 'Moderate', 'Upper Middle', or 'Worst Case'." | |
| ) |
🤖 Prompt for AI Agents
In cea/datamanagement/weather_helper/epwmorpher.py around lines 48-49, the
ValueError message lists snake_case names and "semi_moderate_case" which do not
match the actual accepted pathway strings used by the function; update the
exception text so it lists the exact accepted input values (use the exact
strings from the pathway parsing/mapping in this module) instead of the current
incorrect snake_case list, ensuring the message matches the code’s accepted
inputs verbatim.
| output_directory = os.path.dirname(locator.get_weather_file()) | ||
| shutil.copy(locator.get_weather_file(), os.path.join(output_directory, "before_morph_weather.epw")) | ||
|
|
||
| if not os.path.exists(output_directory): | ||
| os.makedirs(output_directory) | ||
|
|
||
|
|
||
| # 1.2. user_epw_file is specified in the config but defaults to the scenario file | ||
| user_epw_file = locator.get_weather_file() | ||
| if not os.path.exists(user_epw_file): | ||
| raise FileNotFoundError(f"Could not find the specified EPW file: {user_epw_file}") | ||
|
|
There was a problem hiding this comment.
Fix backup ordering and avoid clobbering baseline on re-runs.
You copy weather.epw before ensuring the directory exists and before verifying the file exists, which can raise FileNotFoundError. Also, repeated runs can overwrite the original baseline backup. Ensure the folder exists first, verify the EPW exists, and only create the backup if it’s missing.
Apply this diff:
- output_directory = os.path.dirname(locator.get_weather_file())
- shutil.copy(locator.get_weather_file(), os.path.join(output_directory, "before_morph_weather.epw"))
-
- if not os.path.exists(output_directory):
- os.makedirs(output_directory)
-
-
- # 1.2. user_epw_file is specified in the config but defaults to the scenario file
- user_epw_file = locator.get_weather_file()
- if not os.path.exists(user_epw_file):
- raise FileNotFoundError(f"Could not find the specified EPW file: {user_epw_file}")
+ output_directory = os.path.dirname(locator.get_weather_file())
+ os.makedirs(output_directory, exist_ok=True)
+
+ # 1.2. user_epw_file is specified in the config but defaults to the scenario file
+ user_epw_file = locator.get_weather_file()
+ if not os.path.exists(user_epw_file):
+ raise FileNotFoundError(f"Could not find the specified EPW file: {user_epw_file}")
+
+ # Backup the original weather file only once to preserve the baseline across re-runs
+ backup_epw = os.path.join(output_directory, "before_morph_weather.epw")
+ if not os.path.exists(backup_epw):
+ shutil.copy2(user_epw_file, backup_epw)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| output_directory = os.path.dirname(locator.get_weather_file()) | |
| shutil.copy(locator.get_weather_file(), os.path.join(output_directory, "before_morph_weather.epw")) | |
| if not os.path.exists(output_directory): | |
| os.makedirs(output_directory) | |
| # 1.2. user_epw_file is specified in the config but defaults to the scenario file | |
| user_epw_file = locator.get_weather_file() | |
| if not os.path.exists(user_epw_file): | |
| raise FileNotFoundError(f"Could not find the specified EPW file: {user_epw_file}") | |
| output_directory = os.path.dirname(locator.get_weather_file()) | |
| os.makedirs(output_directory, exist_ok=True) | |
| # 1.2. user_epw_file is specified in the config but defaults to the scenario file | |
| user_epw_file = locator.get_weather_file() | |
| if not os.path.exists(user_epw_file): | |
| raise FileNotFoundError(f"Could not find the specified EPW file: {user_epw_file}") | |
| # Backup the original weather file only once to preserve the baseline across re-runs | |
| backup_epw = os.path.join(output_directory, "before_morph_weather.epw") | |
| if not os.path.exists(backup_epw): | |
| shutil.copy2(user_epw_file, backup_epw) |
🤖 Prompt for AI Agents
In cea/datamanagement/weather_helper/epwmorpher.py around lines 65 to 76, the
code copies the EPW before ensuring the output directory exists and before
checking the file exists, and it always overwrites the baseline backup on
re-runs; fix by (1) compute the weather file path once, (2) ensure
output_directory exists (create it if missing) before any file operations, (3)
check that the weather file exists and raise FileNotFoundError if not, and (4)
create the backup copy only if a backup file (e.g., before_morph_weather.epw)
does not already exist to avoid clobbering the original on repeated runs.
| morphed_data = pyepwmorph_workflow.morph_epw(morph_config.epw, | ||
| morph_config.user_variables, | ||
| morph_config.baseline_range, | ||
| user_future_range, | ||
| year_model_dict, | ||
| [p for p in morph_config.model_pathways if p!="historical"][0], | ||
| user_percentile) | ||
| morphed_data.dataframe['year'] = int(user_future_year) |
There was a problem hiding this comment.
Guard against empty non-historical pathway selection.
Selecting [p for p in morph_config.model_pathways if p != "historical"][0] can raise IndexError if only “historical” is present. Add a safe selection with a clear error.
Apply this diff:
- morphed_data = pyepwmorph_workflow.morph_epw(morph_config.epw,
- morph_config.user_variables,
- morph_config.baseline_range,
- user_future_range,
- year_model_dict,
- [p for p in morph_config.model_pathways if p!="historical"][0],
- user_percentile)
+ pathways = [p for p in morph_config.model_pathways if p != "historical"]
+ if not pathways:
+ raise ValueError("No non-historical pathway available in morph_config.model_pathways")
+ selected_pathway = pathways[0]
+ morphed_data = pyepwmorph_workflow.morph_epw(
+ morph_config.epw,
+ morph_config.user_variables,
+ morph_config.baseline_range,
+ user_future_range,
+ year_model_dict,
+ selected_pathway,
+ user_percentile,
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| morphed_data = pyepwmorph_workflow.morph_epw(morph_config.epw, | |
| morph_config.user_variables, | |
| morph_config.baseline_range, | |
| user_future_range, | |
| year_model_dict, | |
| [p for p in morph_config.model_pathways if p!="historical"][0], | |
| user_percentile) | |
| morphed_data.dataframe['year'] = int(user_future_year) | |
| pathways = [p for p in morph_config.model_pathways if p != "historical"] | |
| if not pathways: | |
| raise ValueError("No non-historical pathway available in morph_config.model_pathways") | |
| selected_pathway = pathways[0] | |
| morphed_data = pyepwmorph_workflow.morph_epw( | |
| morph_config.epw, | |
| morph_config.user_variables, | |
| morph_config.baseline_range, | |
| user_future_range, | |
| year_model_dict, | |
| selected_pathway, | |
| user_percentile, | |
| ) | |
| morphed_data.dataframe['year'] = int(user_future_year) |
🤖 Prompt for AI Agents
In cea/datamanagement/weather_helper/epwmorpher.py around lines 136 to 143, the
code selects the first non-"historical" pathway via [p for p in
morph_config.model_pathways if p!="historical"][0], which will raise IndexError
when only "historical" is present; change this to safely find the first
non-historical pathway (e.g., use next((p for p in morph_config.model_pathways
if p != "historical"), None)) and if none found raise a clear ValueError (or
similar) describing that no non-historical pathway is available and include
morph_config.model_pathways in the message so callers know what was provided;
then use that validated pathway when calling morph_epw.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
cea/datamanagement/weather_helper/epwmorpher.py (2)
80-83: Ensure variables are lists (not scalars) before passing to pyepwmorphCoerce strings to a single-item list to avoid API mismatches.
Apply this diff:
- user_variables = config.weather_helper.variables + user_variables = config.weather_helper.variables + if isinstance(user_variables, str): + user_variables = [user_variables]
98-99: Prefer structured logging over print statementsUse a module logger (logging.getLogger(name)) and log at info/warning levels. Improves observability and integrates with CEA’s logging.
Also applies to: 112-113, 125-126, 138-139, 165-166
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
cea/datamanagement/weather_helper/epwmorpher.py(1 hunks)cea/default.config(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- cea/default.config
🧰 Additional context used
🧬 Code graph analysis (1)
cea/datamanagement/weather_helper/epwmorpher.py (2)
cea/datamanagement/weather_helper/weather_helper.py (1)
main(80-105)cea/inputlocator.py (1)
InputLocator(20-1505)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: tests (windows-latest)
- GitHub Check: tests (macos-latest)
- GitHub Check: tests (ubuntu-latest)
🔇 Additional comments (6)
cea/datamanagement/weather_helper/epwmorpher.py (6)
32-51: Harden scenario parsing and fix misleading error textMake mapping case-insensitive, accept common SSP aliases, and correct the error message to list the actual accepted names.
Apply this diff:
-def convert_scenario_names(name): - """ - Convert the scenario names from CEA to the names used in pyepwmorph - - :param string name: name of the scenario in CEA - :return: name of the scenario in pyepwmorph - :rtype: string - """ - if name == "Best Case": - return "Best Case Scenario" - elif name == "Moderate": - return "Middle of the Road" - elif name == "Upper Middle": - return "Upper Middle Scenario" - elif name == "Worst Case": - return "Worst Case Scenario" - else: - raise ValueError(f"Could not interpret the climate pathway: {name}. " - f"Please choose one of the following: best_case, moderate_case, semi_moderate_case, worst_case") +def convert_scenario_names(name: str) -> str: + """ + Convert a CEA pathway string (or SSP alias) to the pyepwmorph label. + """ + mapping = { + # Best Case (SSP1-2.6) + "best case": "Best Case Scenario", + "best_case": "Best Case Scenario", + "ssp126": "Best Case Scenario", + "ssp1-2.6": "Best Case Scenario", + # Moderate (SSP2-4.5) + "moderate": "Middle of the Road", + "middle of the road": "Middle of the Road", + "middle_of_the_road": "Middle of the Road", + "ssp245": "Middle of the Road", + "ssp2-4.5": "Middle of the Road", + # Upper Middle (SSP3-7.0) + "upper middle": "Upper Middle Scenario", + "upper_middle": "Upper Middle Scenario", + "ssp370": "Upper Middle Scenario", + "ssp3-7.0": "Upper Middle Scenario", + # Worst Case (SSP5-8.5) + "worst case": "Worst Case Scenario", + "worst_case": "Worst Case Scenario", + "ssp585": "Worst Case Scenario", + "ssp5-8.5": "Worst Case Scenario", + } + key = (name or "").strip().lower() + if key in mapping: + return mapping[key] + raise ValueError( + f"Unsupported climate pathway: {name!r}. Choose one of: 'Best Case', 'Moderate', 'Upper Middle', or 'Worst Case'." + )
86-91: Avoid bare broad except; include error detailsCapture the exception for troubleshooting; prefer logging in follow-up.
Apply this diff:
- try: - baseline_range = user_epw_object.detect_baseline_range() - except Exception: - print("Could not detect the baseline range from the EPW file, using default of 1985-2014") - baseline_range = (1985, 2014) # default if the EPW file does not have the years in it + try: + baseline_range = user_epw_object.detect_baseline_range() + except Exception as e: + print("Could not detect the baseline range from the EPW file, using default of 1985-2014") + print(f"Error details: {e}") + baseline_range = (1985, 2014) # default if the EPW file does not have the years in it
16-22: Pin pyepwmorph to ≥0.4.12 and verify Python ≥3.10Recent bugfixes (e.g., DNI handling) landed in 0.4.12. Recommend pinning pyepwmorph>=0.4.12,<0.5.0 and ensuring the project runs on Python ≥3.10. Update your dependency files accordingly.
Based on learnings
#!/bin/bash # Check dependency pins and Python version constraints in the repo set -e echo "=== Search dependency specs for pyepwmorph ===" rg -nP 'pyepwmorph\s*[<>=]' -C2 || true echo echo "=== Check Python versions in workflows/packaging ===" rg -n 'python-version' -g '.github/workflows/*.yml' || true rg -nP '(requires-python|python_requires)' -g '*.{toml,cfg,ini}' || true
66-76: Fix backup ordering and avoid clobbering the baseline on re-runsCopying before ensuring the folder exists can raise FileNotFoundError, and always overwriting the backup loses the baseline. Ensure the folder exists first and only create the backup if it’s missing.
Apply this diff:
- output_directory = os.path.dirname(locator.get_weather_file()) - shutil.copy(locator.get_weather_file(), os.path.join(output_directory, "before_morph_weather.epw")) - - if not os.path.exists(output_directory): - os.makedirs(output_directory) - - - # 1.2. user_epw_file is specified in the config but defaults to the scenario file - user_epw_file = locator.get_weather_file() - if not os.path.exists(user_epw_file): - raise FileNotFoundError(f"Could not find the specified EPW file: {user_epw_file}") + output_directory = os.path.dirname(locator.get_weather_file()) + os.makedirs(output_directory, exist_ok=True) + + # 1.2. user_epw_file is specified in the config but defaults to the scenario file + user_epw_file = locator.get_weather_file() + if not os.path.exists(user_epw_file): + raise FileNotFoundError(f"Could not find the specified EPW file: {user_epw_file}") + + # Backup the original weather file once to preserve the baseline across re-runs + backup_epw = os.path.join(output_directory, "before_morph_weather.epw") + if not os.path.exists(backup_epw): + shutil.copy2(user_epw_file, backup_epw)
139-145: Guard pathway selection against empty resultIndexing [0] after filtering can raise IndexError if only “historical” is present. Select safely and fail with a clear message.
Apply this diff:
- morphed_data = pyepwmorph_workflow.morph_epw(morph_config.epw, - morph_config.user_variables, - morph_config.baseline_range, - user_future_range, - year_model_dict, - [p for p in morph_config.model_pathways if p!="historical"][0], - user_percentile) + pathways = [p for p in morph_config.model_pathways if p != "historical"] + if not pathways: + raise ValueError(f"No non-historical pathway available in morph_config.model_pathways: {morph_config.model_pathways}") + selected_pathway = pathways[0] + morphed_data = pyepwmorph_workflow.morph_epw( + morph_config.epw, + morph_config.user_variables, + morph_config.baseline_range, + user_future_range, + year_model_dict, + selected_pathway, + user_percentile, + )
93-95: Drop redundant int(...) cast config.weather_helper.year is already parsed as an int by IntegerParameter decoding.Likely an incorrect or invalid review comment.
Initial functionality is ready to go! I tested all variables for SSP585 and there were no issues. Most of the integration had to do with renaming and making sure things are in lists instead of scalar. My package uses a lot of lists because it is meant to be iterated, but we turned that functionality off for simplicity in CEA
Summary by CodeRabbit
New Features
Behavior Changes
Chores