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

ENH: add inplace-kwarg to update #21858

Open
h-vetinari opened this issue Jul 11, 2018 · 6 comments
Open

ENH: add inplace-kwarg to update #21858

h-vetinari opened this issue Jul 11, 2018 · 6 comments
Labels
Enhancement inplace Relating to inplace parameter or equivalent Reshaping Concat, Merge/Join, Stack/Unstack, Explode

Comments

@h-vetinari
Copy link
Contributor

h-vetinari commented Jul 11, 2018

Currently, df.update follows the convention of dict.update to return None and update inplace. This is against the prevailing trend (and philosopy?) of pandas to move away from inplace. See for example (one among many...) @TomAugspurger's response in #21841:

Generally, we're moving away from inplace operations. It's confusing whether inplace means no copy or not. Reindex, by definition, can't be inplace unless the index is the same.
We recommend chaining your method calls, and hope to provide better memory control in the future.

The update-method of perfoms an important function regardless of whether it returns the object or None, and so should IMO be enabled to work in chained operations as well.

First step there would be adding an inplace-argument with default True, and then -- potentially -- transitioning with a longish deprecation cycle towards inplace=False.

Relevant xrefs: #21855 #21859

@gfyoung
Copy link
Member

gfyoung commented Jul 12, 2018

I think that's a pretty reasonable suggestion.

cc @jreback @TomAugspurger

@gfyoung gfyoung added the Reshaping Concat, Merge/Join, Stack/Unstack, Explode label Jul 12, 2018
@Liam3851
Copy link
Contributor

Maybe it's just me, but the name update to me connotes inplace-- you're updating the DataFrame. What are you updating if you don't do it inplace? Namewise perhaps the new functionality proposed in #21855 should live in a beefed-up/reworked combine method?

@h-vetinari
Copy link
Contributor Author

@Liam3851
To me, the essence is that df1.update(df2) yields the desired outcome, and whether this object is returned or updated in place is IMO just a semantic distinction.

Python does a lot of inplace stuff (dict.update, list.sort, random.shuffle(list) etc.), and whenever I have to use one of these, I have to break my chain of operations and build around that None-returning function. Maybe that's personal taste, but in the case of df.update, it would be possible to enable both and still cater to the original spirit by having inplace default to True...

@h-vetinari
Copy link
Contributor Author

@gfyoung said:

I think that's a pretty reasonable suggestion.
cc @jreback @TomAugspurger

Re-pinging @jreback @TomAugspurger

@TomAugspurger
Copy link
Contributor

TomAugspurger commented Aug 6, 2018 via email

@h-vetinari
Copy link
Contributor Author

@TomAugspurger

Since you answered by email (and the quoted text does not contain the OP), may I ask: did you read the OP? I did discuss the relationship to dict.update.

Furthermore, adding a kwarg does not necessarily change the default, just provide more options.
Finally, if/once more joins get added to df.update (noted as TODO in the source code itself; see #21855), inplace-ops are even less obviously the best choice.

Alternatively, if update is such a reserved name, one could think of having the required functionality (fusing two dataframes with given precedence and requirement for output dimensions) in a separate funtion called e.g. df.coalesce?

@jbrockmendel jbrockmendel added the inplace Relating to inplace parameter or equivalent label Oct 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement inplace Relating to inplace parameter or equivalent Reshaping Concat, Merge/Join, Stack/Unstack, Explode
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants