-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
Bug groupby idxmin #25531
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 idxmin #25531
Conversation
Codecov Report
@@ Coverage Diff @@
## master #25531 +/- ##
==========================================
+ Coverage 91.75% 91.75% +<.01%
==========================================
Files 173 173
Lines 52960 52962 +2
==========================================
+ Hits 48595 48597 +2
Misses 4365 4365
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #25531 +/- ##
==========================================
+ Coverage 91.26% 91.47% +0.21%
==========================================
Files 172 173 +1
Lines 52965 52875 -90
==========================================
+ Hits 48337 48367 +30
+ Misses 4628 4508 -120
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a whatsnew for 0.24.2?
can you update |
I do not know, how to deal with one of your previous comment that says "this should already be done in _try_cast" . I didn't understand it clearly. I do not know if it should be already there (without my changes) and I should only debug it, or if I should move the logic ( |
I think this would be most logical |
Ouuu, I tried to do it but I found another problem - in |
5f54903
to
510b221
Compare
@rbenes can you have a look thru groupby / datetime issues, I am almost sure this is a duplicate (which this PR would solve) |
@rbenes the last of those issues still persists on me for master - are you sure this doesn't solve that as well? |
@WillAyd @jreback yes guys, you both were right. master
after PR
|
@rbenes great. can you add that as an additional test (and add to the whatsnew note that issue) |
thanks @rbenes |
closes #25444
closes #15306
git diff upstream/master -u -- "*.py" | flake8 --diff