DOC: Added indexing views to roadmap #36988
Conversation
TomAugspurger
added
Roadmap
| >>> df = pd.DataFrame({"A": [1, 2], "B": [3, 4]}) | ||
| >>> df2 = df[["A"]] | ||
| >>> df2.iloc[0, 0] = 10 | ||
| ValueError("error on write to subset of other dataframe") |
There was a problem hiding this comment.
Maybe we should create a new exception type instead of ValueError (with name TBD) ?
There was a problem hiding this comment.
Sure. Will call it MutableViewError, but happy for better names.
There was a problem hiding this comment.
Kind of interesting that numpy uses a ValueError if something isn't writeable:
[ins] In [1]: import numpy as np
[ins] In [2]: arr = np.array([1, 2, 3])
[ins] In [3]: arr.flags.writeable = False
[ins] In [4]: arr[0] = 9
---------------------------------------------------------------------------
ValueError Traceback (most recent call last)
<ipython-input-4-1804f9f074cb> in <module>
----> 1 arr[0] = 9
ValueError: assignment destination is read-only
Possibly even just ReadOnlyError?
| current ``SettingWithCopy`` warning, we would restore the old behavior of silently | ||
| “failing” to update ``df2``. Under Error on Write, we’d track that the ``df2`` created | ||
| by the first getitem references ``df`` and raise an exception when it was being | ||
| mutated. |
There was a problem hiding this comment.
It may be worth including what error-on-write would be "roughly equivalent to" (including comments on each line) to make it clear what would happen if you did break things up like you did in lines 137-139
|
|
||
| .. code-block:: python | ||
|
|
||
| def copy_if_needed(self): |
|
hmm ci/checks are failing |
|
|
||
| # Case 1: user wants mutations of df2 to be reflected in df | ||
| >>> df = pd.DataFrame({"A": [1, 2], "B": [3, 4]}) | ||
| >>> df2 = df[["A"]].as_mutable_view() # name TBD |
There was a problem hiding this comment.
I'm wondering how often users want this behavior? Could be a lack of imagination on my part, but I would expect that the intention is almost always to change only the thing itself when modifying an object.
If it is indeed uncommon then perhaps could simplify things by not making it a part of the API? Users who want to change both objects can still do so without this shortcut (assuming copy on write).
There was a problem hiding this comment.
I personally agree with you. The issue is the inconsistency (today) between doing df["A"] (which is a mutable view - it's a Series) and df[["A"]], which today is not. The proposal is to always make df[["A"]] a view, and do copy-on-write or error-on-write if someone tries to modify it.
|
General question about error / copy on write (forgive me if this has been answered elsewhere), but does the same hold when we have a parent / view but modify the parent instead of the view? For example, supposing there's error on write and we have |
I'm not sure that we have discussed that??? Today the propagation is both ways, when using |
|
Let's try to keep the discussion in the issue. This PR was opened prematurely. The "modifying the parent" issue is mentioned in the proposal at https://github.com/pandas-dev/pandas/pull/36988/files#diff-8a818e230eb272faebe3cf4b7b164a29e0311a42a09a8bd79d17d43d3249f4d3R231. |
This adds the indexing copy vs. view proposal to the roadmap (#36195).
Given the length of the proposal, I've included a summary in the main
roadmap.rst, with a link to the full proposal in a second document. Here's the summary:The major change from the Google doc shared earlier is the recommendation for "Error on Write" over "Copy on Write". I think the proposal should recommend one over the other, and I think "error on write" is probably the way to go, but I'm fairly uncertain on this point.
cc @pandas-dev/pandas-core