[REF] Move comparison methods to EAMixins, share code #21872
Conversation
gfyoung
added
Internals
pandas/_libs/tslibs/period.pyx
Outdated
| @@ -1877,6 +1897,8 @@ def _quarter_to_myear(year, quarter, freq): | |||
| year -= 1 | |||
|
|
|||
| return year, month | |||
| # FIXME: if quarter is None then `month` won't be defined here. | |||
| # just disallow quarter == None? | |||
| return result | ||
|
|
||
| name = '__{name}__'.format(name=op.__name__) | ||
| # TODO: docstring? |
| def _evaluate_compare(self, other, op): | ||
| """ | ||
| We have been called because a comparison between | ||
| 8 aware arrays. numpy >= 1.11 will |
There was a problem hiding this comment.
This is the existing docstring in indexes.datetimelike. I'm open to suggestions.
| # ------------------------------------------------------------------- | ||
| # Shared Constructor Helpers | ||
|
|
||
| def validate_periods(periods): |
Codecov Report
@@ Coverage Diff @@
## master #21872 +/- ##
==========================================
+ Coverage 91.91% 91.92% +<.01%
==========================================
Files 164 164
Lines 49992 50040 +48
==========================================
+ Hits 45952 46001 +49
+ Misses 4040 4039 -1
Continue to review full report at Codecov.
|
pandas/_libs/tslibs/period.pyx
Outdated
|
|
||
| return period_ordinal(year, month, day, hour, | ||
| minute, second, 0, 0, base) | ||
|
|
||
|
|
||
| def _quarter_to_myear(year, quarter, freq): | ||
| def quarter_to_myear(year, quarter, freq): |
There was a problem hiding this comment.
ideally type these, where is quarter passed as None?
There was a problem hiding this comment.
It is used once in period.pyx and once in arrays.period, always with quarter non-None. I'll make that required.
| @@ -21,6 +21,19 @@ | |||
| from pandas.tseries.frequencies import to_offset | |||
|
|
|||
There was a problem hiding this comment.
can you change to absolute imports, I think we are moving away from the relative ones fro readability.
| # Comparison Methods | ||
|
|
||
| @classmethod | ||
| def _add_comparison_methods(cls): |
There was a problem hiding this comment.
so I would define the comparison methods on the class itself, then this should be trivial to change to use the EAMixins
There was a problem hiding this comment.
I agree we should move in this direction, but prefer to do it separately. ATM the PR still broadly resembles cut/paste.
|
Ping. I’d like to get a move on #21907 and the caching issues that will come up with this. |
|
thanks @jbrockmendel |
Changes an old usage numpy's C API that is deprecated. This won't get rid of the warnings because cython hasn't changed it, but still.
Also stops making copies of offsets since they are now immutable.
Handles a handful of changes requested in the last pass: de-privatizes _quarter_to_myear (plus bonus docstring), renames _generate --> _generate_range, comments in is_list_like
renames arrays.timedelta --> arrays.timedeltas to match core.indexes
Implements comparison methods in DatetimeArray and TimedeltaArray, cleans up some Index code that is no longer needed as a result.
Makes some progress on sharing code between TDI and DTI constructors (most of which we want to move up to the array classes)