CLN: Move period.pyx to tslibs/period.pyx #18555
Conversation
pandas/_libs/tslibs/period.pyx
Outdated
| from tslibs.np_datetime cimport (pandas_datetimestruct, | ||
| dtstruct_to_dt64, dt64_to_dtstruct, | ||
| is_leapyear) | ||
| from pandas._libs.tslibs.np_datetime cimport (pandas_datetimestruct, |
There was a problem hiding this comment.
the beauty here is that you can do
from np_datetime cimport .... and so on , the puts period in the same dir as all of the tslibs
pandas/compat/pickle_compat.py
Outdated
| ('pandas.core.indexes.period', 'PeriodIndex') | ||
| ('pandas.core.indexes.period', 'PeriodIndex'), | ||
|
|
||
| # 18543 moving period |
There was a problem hiding this comment.
is there a reason you are changing this? this is a private import and should not be needed
There was a problem hiding this comment.
I saw ('pandas._period', 'Period'): ('pandas._libs.period', 'Period'), on Line 77, and was under the impression I needed to add something for this too.
Although I now realise that even if I did need this code, it would be wrong cause I'd need to modify the value on L77 too.
Would you like me to just remove this new line? and should I modify L77 too?
There was a problem hiding this comment.
hmm ok
why don’t u put the line right under the other one then
There was a problem hiding this comment.
Good point, they seemed to be 'batched' by issue, but it'd be more readable if it was under the original, will move!
|
+1 on this, @AaronCritchley thanks for taking the initiative on this. @jreback thoughts on putting period_helper either in tslibs or tslibs/src? |
move datetime / period to a tslibs/src would be ok i think (but separate / orthogonal PR). |
Codecov Report
@@ Coverage Diff @@
## master #18555 +/- ##
==========================================
- Coverage 91.46% 91.45% -0.02%
==========================================
Files 157 157
Lines 51447 51449 +2
==========================================
- Hits 47055 47051 -4
- Misses 4392 4398 +6
Continue to review full report at Codecov.
|
pandas/_libs/tslibs/period.pyx
Outdated
|
|
||
| cimport util | ||
| from util cimport is_period_object, is_string_object, INT32_MIN | ||
|
|
||
| from missing cimport is_null_datetimelike | ||
| from pandas._libs.missing cimport is_null_datetimelike | ||
| from pandas._libs.tslib import Timestamp |
There was a problem hiding this comment.
change this to
```from timestamps import Timestamp``
There was a problem hiding this comment.
Good spot, I shouldn't have missed this, thanks!
pandas/compat/pickle_compat.py
Outdated
| @@ -74,7 +74,11 @@ def load_reduce(self): | |||
| ('pandas._libs.sparse', 'BlockIndex'), | |||
| ('pandas.tslib', 'Timestamp'): | |||
| ('pandas._libs.tslib', 'Timestamp'), | |||
| ('pandas._period', 'Period'): ('pandas._libs.period', 'Period'), | |||
| ('pandas._period', 'Period'): ('pandas._libs.tslibs.period', 'Period'), | |||
|
|
|||
There was a problem hiding this comment.
put the comment above the period ones (so they are all grouped together)
|
lgtm. ping on green. |
|
@jreback Green! 😄 |
|
thanks @AaronCritchley keep em coming! |
git diff upstream/master -u -- "*.py" | flake8 --diffIs a whatsnew entry needed here?