-
Notifications
You must be signed in to change notification settings - Fork 10
Fluxy #89
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for exporting emissions inventories in Fluxy format as part of the "Fluxy" package. The changes include new tests for the export functionality, a detailed Jupyter notebook for converting EDGAR inventories to Fluxy format, updates in the EDGAR inventory module to ensure the data directory and year are properly set, and the creation of a new module for handling Fluxy exports.
Reviewed Changes
Copilot reviewed 4 out of 6 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
tests/exports/test_fluxy.py | Added tests to validate export_fluxy behavior under various conditions. |
scripts/exports/edgar_2_fluxy.ipynb | Introduced a notebook with examples to convert EDGAR data to Fluxy format. |
emiproc/inventories/edgar.py | Updated to ensure consistent directory creation and initialization of inventory year. |
emiproc/exports/fluxy.py | New module implementing the export of inventories to Fluxy format. |
Files not reviewed (2)
- docs/source/api/exports.rst: Language not supported
- docs/source/models.rst: Language not supported
Comments suppressed due to low confidence (1)
scripts/exports/edgar_2_fluxy.ipynb:121
- The keyword argument 'weigths_file' appears to be misspelled; consider renaming it to 'weights_file'.
remappeds = [remap_inventory(inv, grid, weigths_file=local_dir / ".weights_remap_fluxygrid_2") for inv in invs]
@melodb Would you agree to review this ? mostly because you know fluxy and you can maybe let me know if I did some mistake or anything else |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much for implementing this. It will be very useful! I left some small comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds export support for the fluxy package, including new tests, an example notebook, and associated updates in the edgar inventory and fluxy export modules.
- Added tests in tests/exports/test_fluxy.py to validate export_fluxy functionality.
- Introduced an example notebook (scripts/exports/edgar_2_fluxy.ipynb) demonstrating the conversion from EDGAR inventory to fluxy format.
- Updated edgar and fluxy modules to support directory creation, year assignment, and exporting inventories in Fluxy’s expected format.
Reviewed Changes
Copilot reviewed 4 out of 6 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
tests/exports/test_fluxy.py | Added tests covering failure conditions and successful export of fluxy format. |
scripts/exports/edgar_2_fluxy.ipynb | Notebook example for converting EDGAR data to fluxy format and demonstrating plotting. |
emiproc/inventories/edgar.py | Updated to ensure data directory creation and proper year assignment for inventories. |
emiproc/exports/fluxy.py | Implements export_fluxy for exporting inventory data to Fluxy format using standardized grids. |
Files not reviewed (2)
- docs/source/api/exports.rst: Language not supported
- docs/source/models.rst: Language not supported
Comments suppressed due to low confidence (1)
tests/exports/test_fluxy.py:50
- [nitpick] The test function name 'test_export_flux' is inconsistent with the other tests and may be ambiguous. Consider renaming it to 'test_export_fluxy_success' for clarity.
def test_export_flux():
@melodb I am ready, please check and approve the pr :) |
Adding export for the fluxy package
📚 Documentation preview 📚: https://emiproc--89.org.readthedocs.build/en/89/