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

Exclude None from default_na_values list when reading csv files #374

Merged
merged 8 commits into from Jun 12, 2023

Conversation

rajeee
Copy link
Contributor

@rajeee rajeee commented May 26, 2023

Pandas 2.0 added "None" to default na_values in read_csv().

This causes breaking change from previous version when reading buildstock.csv or other files that has the string "None". This PR adds a custom read_csv function that restores previous behavior. It should also fix #373.

@github-actions
Copy link

github-actions bot commented May 26, 2023

File Coverage
All files 84%
base.py 89%
eagle.py 75%
exc.py 57%
local.py 50%
postprocessing.py 84%
utils.py 96%
sampler/base.py 79%
sampler/downselect.py 33%
sampler/precomputed.py 93%
sampler/residential_quota.py 61%
test/test_docker.py 33%
test/test_validation.py 97%
workflow_generator/base.py 90%
workflow_generator/commercial.py 53%
workflow_generator/residential_hpxml.py 83%

Minimum allowed coverage is 33%

Generated by 🐒 cobertura-action against 4b12209

@rajeee rajeee changed the title Exclude None from default_na list when reading csv files Exclude None from default_na_values list when reading csv files May 27, 2023
@rajeee rajeee requested a review from nmerket May 31, 2023 20:17
Copy link
Member

@nmerket nmerket left a comment

Choose a reason for hiding this comment

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

Looks pretty good. Well done on the testing. Can you add an entry to the changelog?

Also, could you require pandas>2 in setup.py for this? It probably won't work with pandas 1.x anymore.

Comment on lines -37 to +38
wget -q https://github.com/NREL/OpenStudio/releases/download/v3.5.1/OpenStudio-3.5.1+22e1db7be5-Ubuntu-20.04.deb
sudo apt install -y ./OpenStudio-3.5.1+22e1db7be5-Ubuntu-20.04.deb
wget -q https://github.com/NREL/OpenStudio/releases/download/v3.6.1/OpenStudio-3.6.1+bb9481519e-Ubuntu-20.04-x86_64.deb
sudo apt install -y ./OpenStudio-3.6.1+bb9481519e-Ubuntu-20.04-x86_64.deb
Copy link
Member

Choose a reason for hiding this comment

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

Is the new version of OpenStudio required for this? It looks like @joseph-robertson has updated it in #351 as well.

Copy link
Contributor Author

@rajeee rajeee Jun 8, 2023

Choose a reason for hiding this comment

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

Yeah, the develop was missing this update to CI so I did it as a part of this PR. Between Joe's and this, whichever PR gets merged last will have to update from develop (or it might be fine since they are exact same changes).



@staticmethod
def validate_buildstock_csv(project_file, buildstock_df):
Copy link
Member

Choose a reason for hiding this comment

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

What's this doing? Tell me in the changelog.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@rajeee
Copy link
Contributor Author

rajeee commented Jun 8, 2023

Looks pretty good. Well done on the testing. Can you add an entry to the changelog?

Also, could you require pandas>2 in setup.py for this? It probably won't work with pandas 1.x anymore.

Thanks. It should still work for pandas 1.x.

@rajeee rajeee requested a review from nmerket June 8, 2023 21:54
Copy link
Member

@nmerket nmerket left a comment

Choose a reason for hiding this comment

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

Should let it be equal to as well.

setup.py Outdated Show resolved Hide resolved
@nmerket nmerket merged commit 61da0dc into develop Jun 12, 2023
6 checks passed
@nmerket nmerket deleted the csv_reading_fix branch June 12, 2023 14:22
@afontani
Copy link
Collaborator

Thanks, @nmerket and @rajeee!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

residential_quota_downselect leads to bad format buildstock
3 participants