WIP: implement period_helper in cython #19498
Conversation
Codecov Report
@@ Coverage Diff @@
## master #19498 +/- ##
==========================================
- Coverage 91.67% 91.62% -0.06%
==========================================
Files 148 150 +2
Lines 48553 48681 +128
==========================================
+ Hits 44513 44604 +91
- Misses 4040 4077 +37
Continue to review full report at Codecov.
|
| # Conventions | ||
|
|
||
| @cython.cdivision | ||
| cdef inline int get_freq_group_index(int freq) nogil: |
There was a problem hiding this comment.
maybe lets make a pandas._libs/period/.... subdir here rather than trying to jam into tslibs (and move period as well)
There was a problem hiding this comment.
a precursor PR to move period would make this easier
There was a problem hiding this comment.
This is definitely a mess; unfortunately my attempts to port it piece-meal segfaulted all over the place.
But I think this does belong in tslibs. A ton of the code moved from period_helper is redundant with np_datetime, ccalendar (if we get rid of the unused JULIAN_CALENDAR branches), and frequencies.
There was a problem hiding this comment.
@shoyer IIRC you mentioned xarray users needing non-gregorian calendar implementations of... something. Am I remembering that right? If so, is there anything useful in the JULIAN_CALENDAR code for period_helper that should be retained instead of ripped out?
There was a problem hiding this comment.
ok, still make it a subdir, maybe pandas/_lib/tslibs/period then
There was a problem hiding this comment.
I don't think the Julian calendar part is worth keeping around for xarray users.
Climate scientists use non-Gregorian calendars (e.g., a "365 day calendar") to simplify keeping track of seasons in climate models. But I don't think the Julian calendar is particularly interesting here.
|
Closing in favor of #19534 and follow-ups. |
Tests pass locally, haven't run asv yet. Lots of cleanup to do; function names are retained, use camelCase. A bunch of functions overlap with ccalendar (even more so if we remove the unused non-GREGORIAN_CALENDAR case)
git diff upstream/master -u -- "*.py" | flake8 --diff