Conversation
mrocklin
left a comment
mrocklin
left a comment
There was a problem hiding this comment.
Generally this looks good to me.
| if pd.__version__ == '0.18.0': | ||
| from pandas.core import format | ||
| else: | ||
| from pandas.formats import format |
There was a problem hiding this comment.
I think that @jcrist is considering dropping support for Pandas < 0.19
There was a problem hiding this comment.
Yeah, pandas < 0.19 support is going away. I hope to merge that in the next day or two (if all goes well). Could either keep the pandas < 0.19 support, and I'll remove it in that PR, or just wait until that's merged beforehand.
There was a problem hiding this comment.
Support for pandas < 0.19 has been removed, so these version checks can go away :).
sinhrks
force-pushed
the
groupby
branch
2 times, most recently
from
f0c9fd7 to
8002271
Compare
|
What's the status here. Is this good to merge? |
|
cc @jreback , not sure if you're still interested in this problem. |
dask/dataframe/groupby.py
Outdated
| Other keywords passed to groupby | ||
| """ | ||
| def __init__(self, df, index=None, slice=None, **kwargs): | ||
| def __init__(self, df, by=None, slice=None, axis=0, level=None, |
There was a problem hiding this comment.
this is a little clunky to have to name these args (since they are unsupported). Are these remaining kwargs actually used anywhere? Whey not simply list the acceptable ones? (and it will show a TypeError if you pass not named ones)
|
@sinhrks, what's the status of this? Besides removing the pandas version check, and maybe responding to Jeff's request (I'm fine with the way it is, but don't have a strong opinion), this looks good to me. |
|
Let me update to meet @jreback 's request. |
|
Merging soon if no further comments. My apologies for the long delay. |
|
@sinhrks Quick question regarding these changes. I noticed that as part of this PR you are now dropping all kwargs which were previously passed down to the groupby, including |
|
This seems like it would work if there was a single output partition? |
|
But I suppose one could always compute and then sort as well |
6 participants
.info) of BUG: meta-data issue after grouping on latest dask #1844. Also fixedverbose=Falseoutput to be closer to pandas.