-
Notifications
You must be signed in to change notification settings - Fork 20
Develop pandas3 prep #217
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
Develop pandas3 prep #217
Conversation
|
This PR has a bunch of good changes in it, but they're not all pandas related. Let's start with #218 and use these changes in this PR as a starting point for fully supporting pandas 3.0. |
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.
@timcera @PaulDudaRESPEC can we get a couple more sets of eyes on this file? Robert and I talked about this and I left some notes/questions/comments/concerns in #216. I'm not sure why it's in this pandas3 prep PR, maybe Robert can chime in about how it relates to pandas 3 or maybe we should move it to its own PR if it it's important to Robert. Moving this to its own PR could help us all collaborate and understand the need for this file, why it's a good example, and then how to best share/communicate the example to the public in our repo.
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.
@austinorr @timcera @PaulDudaRESPEC
It's a testing file. I know it is useful for development, and having sample dev code synchronously in git record with the developments that are happening IS actually a best practice IMO.
Especially since we are firmly in the development stage, not release stage, trying to eliminate every file that conflicts with goals of directory tree purity seems onerous and discourages development.
|
Disagree on #218 as the path forward. This has useful, easily understandable, non-destructive changes that solve errors, rather than raising a set of minimum versions. It is clean, and ready to commit IMO. |
|
For the record, #218 is just a subset of this PR, not an alternate or conflicting approach. |
| """ | ||
|
|
||
| tsfreq = ts.index.freq | ||
| freq = Minute(siminfo["delt"]) |
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.
Could all these changes be avoided by calling .to_unit(‘ns’) here?
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.
I volunteer to test this alternative so we don’t have to change the primary semantics of our ‘freq’ variable and add a new separate one just to drive resampling.
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.
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.
tests on that PR currently fail on purpose, since pandas<3.0 is not merged yet.
Ready to merge @PaulDudaRESPEC -- This PR restricts non-dev installs to be
< pandas 3.0.0, but adds code that is preliminarily valid for pandas 3.0.0. That is, code the will execute without error under pandas 3.0.0 and and this same code produces identical numerical results to hspf inpandas < 3.0.0. The test coverage that fails when run withpandas-3.0.0will be covered in #209 (Note: the test error IS related to pandas update, and is what I believe to be both an error in the test code, AND a legit simulation difference that needs to be scoped out -- elaborated on in the issue).This PR:
close()method to the custom localHDF5class (yes it is fine to have but no really needed per @austinorrwithstatement inrun()to insure clean handling in the event of an error in runtimeutilities.pyandmain.pyfrom Pandas 3.0, create new HDF5 close() method #216python < 3.11examples/pretest/cmd_regression.pythat creates a brief command prompt friendly tester, as well as leveraging the intermediate products of theRegressTestclass for simulation debugging.RegressTestin the filetests/convert/regression_base.pyto have a silent mode to be a bit more screen friendly (@austinorr this is False by default so no change in previous behavior is made).pandas-3.0.0errorpandas.errors.IndexingError: Unalignable boolean Series provided as indexer (index of the boolean Series and of the indexed object do not match). Again see Verify HSP* and Update RegressTest to run under Pandas 3.0.0 #209THIS IS NOT YET Pandas 3.0 safe. The change to pandas3 are substantial. Most specifically, there have been some things removed from classes, such as the
to_timedelta()method of the.deltaproperty of apandas.timerseries.freq, but most importantly, pandas no longer allowsTimedeltato have things likeY, orMme valid intervals, since they are non-ambiguous. Thus, I am not super optimistic about the IF-THEN constructs in theutilities.pytransform()function continuing to work since it looks at things like the string value of a frequency to see if they have the charactersYandM. The results look OK with test10, but I think we should consider that with more complicated datasets we could definitely encounter timeseries resampling issues. Or maybe not. We can track that in #209