Conversation
There was a problem hiding this comment.
Pull request overview
This pull request addresses EPW file error handling and improves error checking for edge cases in weather file processing and spectral calculations. The changes fix issues #585 and #568 related to EPW file errors and add better error detection mechanisms.
Changes:
- Added validation for empty weather files by checking if any valid data points with GHI > 0 exist after filtering
- Added warning for sub-hourly EPW files suggesting conversion to SAM CSV format
- Fixed tracker angle calculation to properly handle numpy arrays/series returned by pvlib
- Improved error checking for pySMARTS module availability
- Updated spectral_utils.py to consistently return weighted_alb (either the calculated value or None)
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| bifacial_radiance/main.py | Added empty weather file validation in MetObj.init, sub-hourly EPW warning in _readEPW, improved tracker_theta conversion, and pySMARTS import for early error detection |
| bifacial_radiance/spectral_utils.py | Refactored weighted_alb return logic to use explicit else clause for clarity |
| tests/test_bifacial_radiance.py | Added test for empty EPW file exception handling and MET_FILENAME7 constant |
| tests/empty_file.epw | New test file with all GHI values set to 0 to test empty file validation |
| tests/USA_NJ_McGuire.AFB.724096_TMY3.epw | Modified first data row to have 30-minute timestamp for testing sub-hourly detection |
| docs/sphinx/source/whatsnew/pending.rst | Added changelog entry for v0.5.1 documenting bug fixes |
| docs/sphinx/source/whatsnew.rst | Included pending.rst in the changelog |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| metdata2 = demo.readWeatherFile(weatherFile='USA_NJ_McGuire.AFB.724096_TMY3.epw', coerce_year=2000) | ||
| leapday = (metdata2.tmydata.index.month == 2) & (metdata2.tmydata.index.day == 29) | ||
| assert leapday.sum() == 0 |
There was a problem hiding this comment.
The new sub-hourly detection warning added in lines 1516-1518 is not tested. Since the USA_NJ_McGuire.AFB.724096_TMY3.epw file is now modified to contain sub-hourly data (line 9 changed from minute=0 to minute=30), this test should verify that the warning is printed. Consider using pytest's capsys or caplog fixture to capture and assert the warning message is emitted.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…iance into 585_epw_errors
__init__.py