PeriodIndex count & size fix #12874
PeriodIndex count & size fix #12874
Conversation
max-sixty force-pushed the period-index-resample branch from 413fab3 to f7030b4 Compare 
doc/source/whatsnew/v0.18.1.txt Outdated
There was a problem hiding this comment.
more of an explanation for the count issue
max-sixty force-pushed the period-index-resample branch 2 times, most recently from b95c188 to fe1fece Compare 
pandas/tseries/resample.py Outdated
There was a problem hiding this comment.
Delegating the attribute consistency to _shallow_copy here
There was a problem hiding this comment.
yes. (of course be sure to add tests as neeeded, more are better!) and I know you have tests!
There was a problem hiding this comment.
Ha!
This was actually tested in #12771, this is making it more panda-ntic
There was a problem hiding this comment.
I like panda-ntic! (usually I use pandonic)
max-sixty force-pushed the period-index-resample branch from fe1fece to a0fffb6 Compare | end = ax[-1].asfreq(self.freq, how='end') | ||
| values = [] | ||
| else: | ||
| start = ax[0].asfreq(self.freq, how=self.convention) |
There was a problem hiding this comment.
so this doesn't break the test, I suspect if you did how=self.convention it would work no?
or maybe just ignore convention entirely and make:
start = ax[0].asfreq(self.freq, how='start') end = ax[-1].asfreq(self.freq, how='end') There was a problem hiding this comment.
That's the suggestion here, but it breaks that test above. Or am I misunderstanding?
There was a problem hiding this comment.
actually it I think your change makes sense. make a new sub-section on resampling with PeriodIndex (and put your bug fixes there, with a small example which shows this change).
As an aside, I think convention here is really refering to something completely different. IOW it has to do whether to take the start or end of the original series as the counting point for the freq, NOT the period resampling which is independent of that.
There was a problem hiding this comment.
Unless you strongly object I'm going to add an issue on this & follow up; I'm already a bit overextended on this and don't want to leave the stuff completed so far hanging around.
| @jreback green |
| @MaximilianR thanks! I used your defs of |
| @jreback , @MaximilianR : I'm not sure what happened when this PR was committed to |
| I was testing some more things which happened to fail on py3. Reverted for now, the issue is #12866 which is broken. |
| yeah somethings wrong not sure what |
| Just saw this. Are there any more details on the breakage? |
| no this is all fixed though see the linked issue |
| OK, thanks |
* commit 'v0.18.0-114-g6c692ae': (492 commits) BUG: add names parameter to read_excel BUG, DEP, DOC: Patch and Align Categorical's Sorting API TST: remove ResourceWarnings from stat by auto-closing iterator TST: revert some testing additions in pandas-dev#12874, xref pandas-dev#12886 DOC: v0.18.1 corrections BUG: Respect usecols even with empty data PERF: Only do case sensitive check when not lower case BUG: PeriodIndex count & resample-on-same-freq fix DOC: add windows building blog to contributing.rst COMPAT: .query/.eval should work w/o numexpr being installed if possible Implement Akima1DInterpolator DOC: fix code-block ipython highlighting BUG:fixed inconsistent behavior of last_valid_index ENH: Add Index.str.get_dummies TST: Add more Sparse indexing tests ENH: Support CustomBusinessHour BUG: ensure coercing scalars to when setting as numpy BUG: SparseSeries slicing with Ellipsis raises KeyError TST: Add numeric coercion tests BUG: .str methods with expand=True may raise ValueError if input has name ...
3 participants
closes #12774
closes #12868
closes #12770
git diff upstream/master | flake8 --diffInitial attempt at fixing some of the more urgent issues