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

API: ban mutation within groupby.apply #12653

Closed
jreback opened this issue Mar 17, 2016 · 7 comments
Closed

API: ban mutation within groupby.apply #12653

jreback opened this issue Mar 17, 2016 · 7 comments
Labels
API Design Apply Apply, Aggregate, Transform, Map Closing Candidate May be closeable, needs more eyeballs Groupby

Comments

@jreback
Copy link
Contributor

jreback commented Mar 17, 2016

xref #8662
xref #12652

This makes pandas code jump thru hoops and is a complete anti-pattern for pandas. let's nuke it. users should never do this.

@jreback jreback added this to the 0.19.0 milestone Mar 17, 2016
@jreback jreback modified the milestones: 0.20.0, Next Major Release Mar 23, 2017
@jbrockmendel jbrockmendel added the Apply Apply, Aggregate, Transform, Map label Dec 1, 2019
@jbrockmendel
Copy link
Member

agreed this is a footgun. brainstorming how to implement:

  • document that it isnt supported and be done with it
  • maybe a contextmanager to make the relevant objects immutable within the relevant calls

@rhshadrach
Copy link
Member

@jbrockmendel with the contextmanager, are you thinking doing a comparison after the apply call to see if the objects involved changed? Is there a way to avoid a copy here?

@jbrockmendel
Copy link
Member

with the contextmanager, are you thinking doing a comparison after the apply call to see if the objects involved changed?

I am thinking of setting the array.flags.writeable to False for any ndarrays contained inside the DataFrame. We'd need to implement something analogous for ExtensionArrays.

@rhshadrach
Copy link
Member

rhshadrach commented Feb 6, 2021

I think of this issue applying to all UDF functions (apply, aggregate, transform, filter) on all pandas objects. @jbrockmendel - would you support just adding to the docs without the context manager? This would perhaps raise some awareness and I think doesn't require a deprecation, and gives a place we can point to when issues are reported.

@jbrockmendel
Copy link
Member

would you support just adding to the docs without the context manager

I think thats probably our best bet. Something to point out when people inevitably open issues about this.

@rhshadrach
Copy link
Member

I'm +1 on closing this, I don't believe the context manager is necessary.

@rhshadrach rhshadrach added the Closing Candidate May be closeable, needs more eyeballs label Apr 16, 2021
@MarcoGorelli
Copy link
Member

Looks like this has been clearly documented in https://pandas.pydata.org/pandas-docs/stable/user_guide/gotchas.html#gotchas-udf-mutation by #39762 - closing then, but please let me know if I've misunderstood, or if you believe that just documenting this isn't enough of a solution

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Design Apply Apply, Aggregate, Transform, Map Closing Candidate May be closeable, needs more eyeballs Groupby
Projects
None yet
Development

No branches or pull requests

4 participants