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
Conversation
Minimum allowed coverage is Generated by 🐒 cobertura-action against 4b12209 |
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.
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.
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 |
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.
Is the new version of OpenStudio required for this? It looks like @joseph-robertson has updated it in #351 as well.
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.
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): |
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.
What's this doing? Tell me in the changelog.
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.
Done.
Thanks. It should still work for pandas 1.x. |
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.
Should let it be equal to as well.
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.