Skip to content
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

GREET Data Hotfix #408

Merged
merged 2 commits into from
Dec 5, 2024
Merged

GREET Data Hotfix #408

merged 2 commits into from
Dec 5, 2024

Conversation

dakotaramos
Copy link
Collaborator

Hotfix to correct values and units for iron ore mining and pelletizing emission intensities based on feedback from ANL.

Related issue

#380

Impacted areas of the software

resources

Additional supporting information

This work is being completed for the LCA integration, the data pulling and parsing will live in HOPP, the LCA calculations with that data will live in GreenHEART

Test results, if applicable

tests/hopp/test_greet_resource.py ...

@dakotaramos dakotaramos added bug Something isn't working high priority Need to tackle soon labels Dec 4, 2024
@dakotaramos dakotaramos self-assigned this Dec 4, 2024
@johnjasa
Copy link
Collaborator

johnjasa commented Dec 5, 2024

Thanks for this, @dakotaramos! The changes you've made look good.
The failing CI appears unrelated to your changes. @RHammond2, could you please take a look and see if a package update or similar has caused these tests to fail?

@RHammond2
Copy link
Collaborator

Thanks for this, @dakotaramos! The changes you've made look good. The failing CI appears unrelated to your changes. @RHammond2, could you please take a look and see if a package update or similar has caused these tests to fail?

The issue is stemming from the KML object having features as a callable, so if we change these lines to use k.features instead of k.features(), then we should be ok in theory.

However, something I noticed is that we'll also be attempting to index an empty list in the tests, so this will raise a new error. Doubly however, this is call was not previously an issue, so I checked out the fastkml changelog, and they had a v1.0 release in mid-November.

tl;dr: Pin fastkml to be < 1, and open an issue to migrate HOPP and GreenHEART to be compatible with fastkml v1.0.

Copy link
Collaborator

@RHammond2 RHammond2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside from the fastkml issue in my comment this all looks good to me.

@dakotaramos
Copy link
Collaborator Author

@RHammond2 @johnjasa just pushed the update to the fastkml version in pyproject.toml

@johnjasa johnjasa merged commit 6210157 into NREL:develop Dec 5, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working high priority Need to tackle soon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants