Skip to content

Commit 1a4605d

Browse files
Update PDEP8 text around open questions
1 parent eb4f6f8 commit 1a4605d

File tree

1 file changed

+15
-11
lines changed

1 file changed

+15
-11
lines changed

web/pandas/pdeps/0008-inplace-methods-in-pandas.md

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ Summarizing for the `inplace` keyword, we propose to:
255255
(group 4) or only update the object (group 3, "object-inplace", which can be emulated
256256
with reassigning).
257257

258-
### Open Questions
258+
### Other design questions
259259

260260
#### With `inplace=True`, should we silently copy or raise an error if the data has references?
261261

@@ -290,12 +290,13 @@ lazy copy" to be triggered under Copy-on-Write. It is also hard to fix, adding a
290290
with `inplace=True` might actually be worse than triggering the copy under the hood. We would only copy columns that
291291
share data with another object, not the whole object like `.copy()` would.
292292

293-
There is another possible variant, which would be to trigger the copy (like the first option), but have an option to
294-
raise a warning whenever this happens.
293+
**Therefore, we propose to silently copy when needed.** The `inplace=True` option would thus mean "try inplace whenever possible", and not guarantee it is actually done inplace.
294+
295+
In the future, if there is demand for it, it could still be possible to add to option to raise a warning whenever this happens.
295296
This would be useful in an IPython shell/Jupyter Notebook setting, where the user would have the opportunity to delete
296297
unused references that are causing the copying to be triggered.
297298

298-
For example,
299+
For example:
299300

300301
:::ipython
301302
In [1]: import pandas as pd
@@ -334,16 +335,16 @@ was not inplace, since it is possible to go out of memory because of this.
334335

335336
#### Return the calling object (`self`) also when using `inplace=True`?
336337

337-
The downsides of keeping the `inplace=True` option for certain methods, are that the return type of those methods will
338-
now depend on the value of `inplace`, and that method chaining will no longer work.
339-
340-
One way around this is to have the method return the original object that was operated on inplace when `inplace=True`.
338+
One of the downsides of the `inplace=True` option is that the return type of those methods
339+
depends on the value of `inplace`, and that method chaining does not work.
340+
Those downsides are still relevant for the cases where we keep `inplace=True`.
341+
To address this, we can have those methods return the object that was operated on
342+
inplace when `inplace=True`.
341343

342344
Advantages:
343345

344346
- It enables to use inplace operations in a method chain
345347
- It simplifies type annotations
346-
- It enables to change the default for `inplace` to True under Copy-on-Write
347348

348349
Disadvantages:
349350

@@ -352,8 +353,11 @@ Disadvantages:
352353
returned (`df2 = df.method(inplace=True); assert df2 is df`)
353354
- It would change the behaviour of the current `inplace=True`
354355

355-
Given that `inplace` is already widely used by the pandas community, we would like to collect feedback about what the
356-
expected return type should be. Therefore, we will defer a decision on this until a later revision of this PDEP.
356+
We generally assume that changing to return `self` should not give much problems for
357+
existing usage (typically, the current return value of `None` is not actively used).
358+
Further, we think the advantages of simplifing return types and enabling methods chains
359+
outweighs the special case of returning an identical object.
360+
**Therefore, we propose that for those methods with an `inplace=True` option, the calling object (`self`) gets returned.**
357361

358362
## Backward compatibility
359363

0 commit comments

Comments
 (0)