(WIP) ENH: Add PeriodBlock #13755
(WIP) ENH: Add PeriodBlock #13755
Conversation
sinhrks added Enhancement Performance 
Current coverage is 85.51% (diff: 84.37%)@@ master #13755 diff @@ ========================================== Files 145 145 Lines 51404 51655 +251 Methods 0 0 Messages 0 0 Branches 0 0 ========================================== + Hits 43975 44172 +197 - Misses 7429 7483 +54 Partials 0 0
|
pandas/core/internals.py Outdated
There was a problem hiding this comment.
you might be better actoring out some of the holding Index suff from DatetimeTZBLock and creating IndexHolderBlock (or better name). then sub-classing for DatetimeTZBlock and PeriodBlock. Should be in theory less duplication.
sinhrks force-pushed the periodblock6 branch 4 times, most recently from dc92e4a to 8b5ea28 Compare pandas/core/internals.py Outdated
sinhrks force-pushed the periodblock6 branch 2 times, most recently from bcfb433 to 5596c3d Compare pandas/core/reshape.py Outdated
There was a problem hiding this comment.
I guess not handling other i8 here ATM? (e.g. datetime/datetimetz)?
| @sinhrks looks pretty good. certainly can merge mostly as is and just make a to-do on other issues. Looks like pretty much works and doesn't change the api much. Just on a small whatsnew section to we can keep updating. |
sinhrks force-pushed the periodblock6 branch 2 times, most recently from 661e4d2 to 8d8ef2f Compare 
| ideally I'd like this to go in. I think most of the prep work has been done. |
sinhrks force-pushed the periodblock6 branch from 321e337 to 3cdb115 Compare | @sinhrks Is this ready for review? (no docs yet, and some todo's listed in the code) Just to prioritize my time :-) |
| Not yet. I'll make a notice once ready, hopefully this weekend... :) |
sinhrks force-pushed the periodblock6 branch from 0ac8737 to e57c684 Compare | self.is_categorical = None | ||
| self.is_period = None | ||
| if values.ndim == 1: | ||
| if isinstance(values, Categorical): |
| # period | ||
| df = pd.DataFrame({'a': pd.period_range('2013', freq='M', periods=3)}) | ||
| self.check_error_on_write(df, ValueError) | ||
| self.check_error_on_write(df, feather.FeatherError) |
There was a problem hiding this comment.
yeah I don't think feather supports Periods ATM.
| if k != self.table.n_buckets: | ||
| val = self.table.vals[k] | ||
| | ||
| @cython.wraparound(False) |
There was a problem hiding this comment.
this file changes can do as a separate PR?
| index = values | ||
| | ||
| if is_period_arraylike(index): | ||
| if isinstance(index, pd.PeriodIndex): |
| from pandas.tseries.period import Period | ||
| if isnull(fill_value): | ||
| fill_value = tslib.iNaT | ||
| elif isinstance(fill_value, Period): |
There was a problem hiding this comment.
you can use ABCPeriod here (or is_period) maybe better
| | ||
| def is_period(array): | ||
| """ return if we are a period array """ | ||
| return isinstance(array, ABCPeriodIndex) or is_period_arraylike(array) |
There was a problem hiding this comment.
hmm so this is now the same as is_period_dtype, maybe we should just deprecate this then, unless this is really equiv to isinstance(val, Period) ? IOW a scalar
| let's close for now. If you want to rebase and fixup lmk. Would like to do this (maybe in concert with a |
4 participants
git diff upstream/master | flake8 --diffThis PR is NOT ready for review, but to show basic direction. Especially, supporting period arithmetic is quite unreliable because current
_TimeOpclass is basically fornp.ndarrayoperations. I think refactoring_TimeOpis needed prior to this PR.Would like to split the PR to 3 parts:
PeriodDtypedefinition, letPeriodIndexto havePeriodDtypelikeperiod[M]. (ENH: PeriodIndex now has period dtype #13941)_TimeOpsplitting to_DatetimeOpand_TimedeltaOp, mergingDatetimeIndexandTimedeltaIndexarithmetics._DatetimeOpis for lhs or rhs isdatetime64ordatetimetzdtypes._TimedeltaOpis for othertimedeltarelated types.PeriodBlock, adding_PeriodOpfor arithmetic (this PR).