parse: Support pandas 2.x #1436
Conversation
Replace parse_time_string() by parse_datetime_string_with_reso() to gain pandas 2.0 compatitibility. This change was hinted by s3v in [Debian bug #1044079] and allows the resolution of the github issue nextstrain#1303 appropriately. [Debian bug #1044079]: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1044079#46 Thanks: s3v <c0llapsed@yahoo.it> Signed-off-by: Étienne Mollier <emollier@debian.org>
There was a problem hiding this comment.
@emollier thanks for the PR. We have no current plans to support pandas version 2 and will continue to pin to 1.x for PyPI/Bioconda until we're ready to support 2.x in all other parts of the codebase.
In the meantime, this specific call can be made compatible with both versions for environments that allow more flexible dependency versions.
The initial pandas 2.x change does not work with pandas 1.x. In 1.x, parse_datatime_string_with_reso is a cdef function which cannot be called ouside of the Cython module. CI error:
E AttributeError: module 'pandas._libs.tslibs.parsing' has no attribute 'parse_datetime_string_with_reso'
Trapping AttributeError and falling back to code written for pandas 1.x should work.
Co-authored-by: Victor Lin <13424970+victorlin@users.noreply.github.com>
|
Thank you for your time and your review, I took some time to test it in various conditions to make sure it works as expected, and saw no issues indeed. Even if you keep the pin to pandas 1.x, this may facilitate the change to pandas 2.x series when you feel that you are ready for it.
I very much agree about an implementation using the public API: this would avoid further risks of abrupt breakage without notice or guidance from pandas. I'm afraid however that I'm not comfortable enough with augur's field of application nor pandas itself to be able to propos such a change myself. :/ |
|
@emollier thanks for the update! I've just approved the test runs. Once they pass, I'll add a changelog entry and get this merged. |
|
You're welcome, |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1436 +/- ##
==========================================
+ Coverage 68.66% 68.67% +0.01%
==========================================
Files 69 69
Lines 7554 7557 +3
Branches 1851 1851
==========================================
+ Hits 5187 5190 +3
Misses 2089 2089
Partials 278 278 ☔ View full report in Codecov by Sentry. |
Replace parse_time_string() by parse_datetime_string_with_reso() to gain pandas 2.0 compatitibility.
This change was hinted by s3v in Debian bug #1044079 and allows the resolution of the github issue #1303 appropriately.
Thanks: s3v c0llapsed@yahoo.it
PS: filling the mandatory form below for your convenience.
Description of proposed changes
This change modifies augur/parse.py by replacing parse_time_string() by parse_datetime_string_with_reso() in order to gain pandas 2.0 compatitibility.
Related issue(s)
#1303
Checklist