Closed jorisvandenbossche closed 1 year ago
:+1: nice list
Question: Are all of the items on the SeriesGroupBy
and DataFrameGroupBy
whitelists meant to pass through to the underlying Series
and DataFrame
members? From what I can tell there are many items on the respective whitelists that are handled before they can ever be so dispatched. Here is a list:
first
, last
, min
, max
, sum
- all defined as members in the GroupBy
class definition (using _groupby_function
). (GroupBy.prod
is also defined in this way, but is not whitelisted.) And boxplot
is also a defined member of DataFrameGroupBy
(but not SeriesGroupBy
).
count
, cumcount
, head
, tail
, mean
, median
- all defined, with def
keyword, in the GroupBy
class definition.
I ask because I'm proposing to define the whitelisted methods at class definition time, instead of relying on the GroupBy.__getattr__
to dynamically create them at the time they are invoked. And so sphinx will generate the documentation for all passed-through methods just like the ones that have first-class definitions in the Class.
But that often overrides the explicit method definitions listed above. And sometimes they don't even do the same thing. Which makes me suspect that these dozen or so whitelisted methods no longer really should be handled in this way (i.e. they already have explicit class definitions that take priority.)
So, can anyone think of a reason why we shouldn't just remove these names from the {Series,DataFrame}GroupBy
whitelists? If not, I'll make a pull request addressing #6944 that uses this as part of the solution (with an appropriately updated test_groupby:test_groupby_whitelist
.)
by definition the cythonized functions (and a few others which sometimes be cythonized) are defined inline on the groupby objects (eg min,mean,sum etc) so these are not passed thru
you can define the whitelist methods in the classes (and just have the getattr raise an error I suppose) (and this define the doc strings) - if a method exists in the class it is called before getattr is ever called so of course these override the whitelist
prod is prob an error that it is not defined some methods only work in frames and not on series so that is done on purpose (boxplot)
I think this will be easier with Python3 since we can rewrite function signatures without much hassle (something like https://github.com/pandas-dev/pandas/blob/08a66e111a60b9a25fb59a0dcc691c3f1ec0ad1e/pandas/util/_decorators.py#L128). Then we can get proper signatures instead of **kwargs
.
CAN I HELP? (as former professor, good at scrutinizing long and "boring" documents.) Hi, everybody,
my name is Lucas. I have a PhD in development economics and "greener" python enthusiast. Unfortunately, I don't have the time just right now to look at @jorisvandenbossche comments with the care that they deserve, but I did want to make my own remark on something I noticed that probably needs a small adjustment (I'm also sorry for not having had time to read the other posts, which i intend to do latter.).
At Pandas' website page "pandas.core.groupby.GroupBy.agg", which seems to mee to be one of great importance to people starting to wrap their minds around python's way of referencing and slicing different ranges, there is no mention about how the code works. By glancing over the source code for the funcion, i noticed 'agg' is short for 'aggregate'. But, since people over the web seems to be using the short version of the method, wouldn't it be nice to have a redirectioning link for new students? Moreover, i also noticed that the following section of the website, titled "pandas.core.groupby.SeriesGroupBy.aggregate", contains info that seems to be relevant to the previous section.
This seems to have some connection with the third issue mentioned by , which is: "put all relevant docstrings (...) eg now the aggregate (...) docstrings of GroupBy are empty, but are more elaborate in the subclasses)". Now, I still am not very good at coding, but, as a former college professor in Brazil, I can say with relative confidence that i am good at reading what most people consider "boring" stuff, and at finding small things others don't give attention to. That being said, I would like to know how can i help on this issue? Thank you. Have a nice day!
It appears most of the issues in the original post are addressed so closing. If there are specific groupby doc issues, it would be better to open specific issues
An overview of the reference doc on groupby is given here: http://pandas.pydata.org/pandas-docs/dev/api.html#groupby (apart from the extensive user guide: http://pandas.pydata.org/pandas-docs/dev/groupby.html)
There are some things that could use some improvement:
first
/last
/nth
count
,cumcount
, ..name
: not sure what the purpose of this isGroupBy
object itself to the api docs (and so automatically all its methods) (#19302)aggregate
andtransform
docstrings of GroupBy are empty, but are more elaborate in the subclasses) (#8231)apply
docstring is not very clear to meby
arg (and provide some short examples in the 'Examples' section)_apply_whitelist
g = df.groupby(...); g.count?
is returning<no docstring>
(see https://github.com/pydata/pandas/issues/4500#issuecomment-41220138 for explanation how)head/tail/nth
are basicallyfilter
type of functions,fillna/shift
are transformers, while almost everything else is a reducer (e.g.sum/mean/describe
), whileapply/agg
can be any of the above. hmm. maybe needs a separate section for this. (and of courseas_index
just makes this crazy)If someone wants to tackle this (or parts of this), go ahead!