Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: groupby's first/last functions maintain Series rather than convert to numpy array (#15884) #15885

Closed
wants to merge 1 commit into from

Conversation

chrisaycock
Copy link
Contributor

@jreback
Copy link
Contributor

jreback commented Apr 4, 2017

@chrisaycock not what I was looking for. The only reason that compat routine is called is because the block aggregator raises. So this will not be performant. I don't think these compat routines are actually called at all (or should be). All of the handling is now done via the cython routines (I *think).

@jreback
Copy link
Contributor

jreback commented Apr 4, 2017

So these compat routines are only called if numeric_only=False is passed. It tries the cython path first. So for datetimes it should be working (but is raising because its being called incorrectly).

@chrisaycock
Copy link
Contributor Author

@jreback Those compat functions were definitely being called for the test case, which is how I found them. If they aren't supposed to be called, then I'll have to take another crack at that issue.

@jreback
Copy link
Contributor

jreback commented Apr 4, 2017

@chrisaycock not what I mean.

put a breakpoint in _cython_agg_blocks. It is hit first (pun intended). This raises (incorrectly), then the compat routines are called.

@chrisaycock
Copy link
Contributor Author

@jreback Ok, I see what you mean. The .values is definitely a DatetimeIndex instead of a multi-dimensional array.

@chrisaycock chrisaycock closed this Apr 4, 2017
@chrisaycock chrisaycock deleted the GH15884 branch April 4, 2017 00:58
@codecov
Copy link

codecov bot commented Apr 4, 2017

Codecov Report

Merging #15885 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #15885      +/-   ##
==========================================
- Coverage   90.97%   90.96%   -0.01%     
==========================================
  Files         145      145              
  Lines       49483    49481       -2     
==========================================
- Hits        45015    45012       -3     
- Misses       4468     4469       +1
Flag Coverage Δ
#multiple 88.72% <100%> (-0.01%) ⬇️
#single 40.63% <0%> (ø) ⬆️
Impacted Files Coverage Δ
pandas/core/groupby.py 95.02% <100%> (-0.01%) ⬇️
pandas/core/common.py 90.68% <0%> (-0.35%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update da0523a...c0d29ff. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: first() loses the timezone in groupby
2 participants