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

Change nan behavior #121

Merged
merged 8 commits into from
Sep 8, 2023
Merged

Change nan behavior #121

merged 8 commits into from
Sep 8, 2023

Conversation

paulf81
Copy link
Collaborator

@paulf81 paulf81 commented Sep 6, 2023

Ready to be merged (maybe can delete some commented out lines)

Feature or improvement description
This pull request address Issue #119 , where we inadvertently changed nan filtering from an "Any" to an "All" in the conversion of energy ratio from pandas to polars. This pull requests fixes the convention back, and adds a test to confirm the expected behavior (and check going forward)

Related issue, if one exists
#119

Impacted areas of the software
energy_ratio.py
energy_ratio_output.py
energy_ratio_utiltities.py
energy_ratio_test.py

@paulf81 paulf81 added the bug Something isn't working label Sep 6, 2023
@paulf81 paulf81 added this to the v1.4 milestone Sep 6, 2023
@paulf81 paulf81 self-assigned this Sep 6, 2023
@paulf81
Copy link
Collaborator Author

paulf81 commented Sep 8, 2023

@misi9170 good to merge?

@misi9170
Copy link
Collaborator

misi9170 commented Sep 8, 2023

@misi9170 good to merge?

@paulf81
I've made some updates here based around allowing users to choose whether to use "all" or "any" behavior. In doing so, I decided to move the filtering steps to utilities.

I also made an unrelated to change to have the process of checking inputs to compute_energy_ratio() happen in a separate utility for ease of reading. Feel free to revert that if you disagree.

@paulf81
Copy link
Collaborator Author

paulf81 commented Sep 8, 2023

Looks great @misi9170, thank you! Merging

@paulf81 paulf81 merged commit 87eb0fe into NREL:develop Sep 8, 2023
2 checks passed
@paulf81 paulf81 deleted the change_nan_behavior branch September 8, 2023 17:45
@misi9170 misi9170 mentioned this pull request Oct 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants