Doc: Add warning to treat group chunks as immutable #19175
Conversation
pdpark
commented
Jan 10, 2018
pdpark
commented
- closes DOC: clarify dangers of fast apply in GroupBy apply docs #14180
|
New clean pull request with only the changes for this issue. |
doc/source/groupby.rst
Outdated
| @@ -944,13 +944,17 @@ that is itself a series, and possibly upcast the result to a DataFrame: | |||
| So depending on the path taken, and exactly what you are grouping. Thus the grouped columns(s) may be included in | |||
| the output as well as set the indices. | |||
|
|
|||
| .. warning:: | |||
| .. warnings:: | |||
doc/source/groupby.rst
Outdated
| first group to decide whether it can take a fast or slow code | ||
| path. This can lead to unexpected behavior if func has | ||
| side-effects, as they will take effect twice for the first | ||
| group. | ||
|
|
||
| * Apply should not perform in-place operations on the group chunk. |
Codecov Report
@@ Coverage Diff @@
## master #19175 +/- ##
=======================================
Coverage 92.19% 92.19%
=======================================
Files 169 169
Lines 50959 50959
=======================================
Hits 46980 46980
Misses 3979 3979
Continue to review full report at Codecov.
|
|
Requested changes made. |
|
@pandas-dev/pandas-core if someone wants to have a look here |
|
As discussed in one of the other issues (#14927), I think we should rather deprecate the special cased behaviour when the group is mutated, than documenting it. |
If you mean "the return value of But my understanding is that the actual changes to the original Did I misunderstand? If not, maybe the warning can be made more specific. |
Ah, but I was not talking about changes to the original dataframe. Does that ever happen? (that would mean the group is a view on the original dataframe). I would like to see a code example for that. What I thought it was about that mutating the group (inplace), so the same object is returned from the function, leads to a different behaviour of how the results are back combined together. |
To be honest I have no idea. If it does not happen, we might then say (after fixing #14927 ) that mutating the chunks can be freely done, because/but it won't affect the original df. (if it can possibly happen, we might want to keep the warning, and make it refer specifically to the original df) |
pdpark
force-pushed
the
doc-groupby-14180
branch
from
4800ea6 to
b98e0ce
Compare
|
How does this look? |
|
@toobaz @jorisvandenbossche are you happy to merge the latest version of this, or should we close the PR instead? |
|
Closing this if it's not going to be merged, feel free to reopen if anyone is interested in this. |
