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

Fixing error/warnings when writing to HDF5, circleRadius bug. #724

Merged
merged 2 commits into from
Nov 10, 2023

Conversation

astronomerritt
Copy link
Collaborator

@astronomerritt astronomerritt commented Nov 8, 2023

Fixes #722.
Fixes #727

The NaturalNameWarning is now suppressed when writing to HDF5 when your object IDs start with a number (or don't otherwise fit PyTables' rather strict naming pattern ^[a-zA-Z_][a-zA-Z0-9_]*$). Unless the user is going to be using PyTables to look at the data, this won't be an issue - we can warn them about this in the docs.

While recreating this error, I also found a bug. At some point in the code -- I have no idea why or where, grepping didn't help -- the ObjID column is being changed to a Pandas dtype that PyTables doesn't support ('StringDtype'), leading to errors when writing out to HDF5.

This wasn't caught in the unit test because the unit test for PPOutWriteHDF5() just loads in some data from a CSV file then writes it out. Short of running all of Sorcha in test_PPOutWriteHDF5(), I'm not sure this COULD have been caught - might be an argument for having end-to-end tests for all the output formats.

I have inserted a line in PPOutWriteHDF5() that changes the type of that column to a standard 'str' instead.

ADDITIONAL FIX:
I have also fixed that one horrible lonely instance of circleRadius being used instead of circle_radius in PPPrintConfigsToLog(). Finally.

Review Checklist for Source Code Changes

  • Does pip install still work?
  • Have you written a unit test for any new functions?
  • Do all the units tests run successfully?
  • Does Sorcha run successfully on a test set of input files/databases?
  • Have you used black on the files you have updated to confirm python programming style guide enforcement?

Copy link

codecov bot commented Nov 8, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Files Coverage Δ
src/sorcha/modules/PPOutput.py 84.78% <100.00%> (+1.44%) ⬆️
src/sorcha/modules/PPConfigParser.py 66.95% <0.00%> (ø)

📢 Thoughts on this report? Let us know!

@astronomerritt astronomerritt changed the title Fixing error/suppressing warnings when writing to HDF5. Fixing error/warnings when writing to HDF5, circleRadius bug. Nov 8, 2023
@astronomerritt astronomerritt merged commit c766c71 into dirac-institute:main Nov 10, 2023
10 checks passed
@astronomerritt astronomerritt deleted the pytables_warning branch January 10, 2024 12:14
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.

Circle radius issue NaturalNameWarning if using HDF5
2 participants