Proposal for future copy / view semantics in indexing operations

[TomAugspurger]

Update: concrete proposal for Copy-on-Write: https://docs.google.com/document/d/1ZCQ9mx3LBMy-nhwRl33_jgcvWo9IWdEfxDNQ2thyTb0/edit?usp=sharing

---

[original post]

Hi all,

During the recent sprint we discussed copy / view semantics in indexing operations. I've tried to summarize a proposal that came out of it.

The tldr is

1. Indexing operations always return a view where possible (always when indexing columns, if possible when indexing the rows).
2. For consistent behavior, we implement either Copy on Write or Error on Write (explained in the proposal)
3. For readability / understandability, we give users the tools to control in code whether mutations to a "child" dataframe (result of an indexing operation) propagate to the parent, while avoiding unnecessary "defensive" copies.

The full proposal is available at https://docs.google.com/document/d/1csGE4qigPR2vzmU2--jwURn3sK5k5eVewinxd8OUPk0/edit. I'd like to collect comments there and then propose properly by adding it to the roadmap.

cc @pandas-dev/pandas-core

[jbrockmendel]

asv from a branch that changes column-based indexing to always return a view:

       before           after         ratio
     [42289d0d]       [f99abd57]
     <master>         <myway>   
-     9.24±0.06ms      8.77±0.02ms     0.95  algorithms.Hashing.time_series_string
-        57.3±2ms       54.3±0.2ms     0.95  join_merge.MergeAsof.time_by_int('backward', 5)
-      76.1±0.3ms       72.0±0.3ms     0.95  frame_methods.Describe.time_dataframe_describe
-        57.3±1ms       54.0±0.2ms     0.94  join_merge.MergeAsof.time_by_int('backward', None)
-     14.0±0.07ms      13.2±0.08ms     0.94  groupby.MultiColumn.time_cython_sum
-        1.29±0ms      1.20±0.01ms     0.94  timeseries.AsOf.time_asof_nan_single('DataFrame')
-     1.50±0.01ms      1.38±0.01ms     0.92  timeseries.AsOf.time_asof_single('DataFrame')
-     3.62±0.01ms      3.31±0.01ms     0.91  groupby.CountMultiDtype.time_multi_count
-      67.0±0.4ms       60.7±0.4ms     0.91  reshape.Crosstab.time_crosstab_normalize_margins
-      14.8±0.4ms      13.2±0.08ms     0.89  join_merge.MergeAsof.time_on_int32('nearest', 5)
-     14.4±0.09ms      12.8±0.04ms     0.89  join_merge.MergeAsof.time_on_int32('nearest', None)
-      14.7±0.1ms      13.0±0.06ms     0.89  join_merge.MergeAsof.time_on_uint64('nearest', 5)
-     14.3±0.05ms      12.6±0.03ms     0.88  join_merge.MergeAsof.time_on_uint64('nearest', None)
-      69.8±0.5ms       61.4±0.7ms     0.88  reshape.PivotTable.time_pivot_table_margins
-     14.3±0.06ms       12.6±0.1ms     0.88  join_merge.MergeAsof.time_on_int('nearest', 5)
-     12.7±0.05ms      11.1±0.04ms     0.87  join_merge.MergeAsof.time_on_int32('forward', 5)
-     14.0±0.06ms      12.3±0.05ms     0.87  join_merge.MergeAsof.time_on_int('nearest', None)
-     12.5±0.05ms      10.9±0.05ms     0.87  join_merge.MergeAsof.time_on_int32('forward', None)
-     12.1±0.05ms      10.5±0.02ms     0.87  join_merge.MergeAsof.time_on_int32('backward', 5)
-     12.0±0.05ms      10.3±0.04ms     0.86  join_merge.MergeAsof.time_on_int32('backward', None)
-     12.7±0.09ms      10.9±0.04ms     0.86  join_merge.MergeAsof.time_on_uint64('forward', 5)
-     12.2±0.06ms      10.5±0.07ms     0.85  join_merge.MergeAsof.time_on_int('forward', 5)
-     12.4±0.03ms      10.6±0.05ms     0.85  join_merge.MergeAsof.time_on_uint64('forward', None)
-     12.0±0.06ms      10.2±0.04ms     0.85  join_merge.MergeAsof.time_on_int('forward', None)
-     11.9±0.05ms      10.1±0.05ms     0.85  join_merge.MergeAsof.time_on_uint64('backward', 5)
-     11.8±0.04ms      9.98±0.05ms     0.85  join_merge.MergeAsof.time_on_uint64('backward', None)
-      11.4±0.4ms      9.61±0.02ms     0.84  join_merge.MergeAsof.time_on_int('backward', None)
-     11.5±0.06ms      9.71±0.04ms     0.84  join_merge.MergeAsof.time_on_int('backward', 5)
-         407±3μs          291±2μs     0.72  reindex.Reindex.time_reindex_columns
-      3.99±0.1ms       2.72±0.2ms     0.68  arithmetic.MixedFrameWithSeriesAxis.time_frame_op_with_series_axis1('ge')
-     3.56±0.02ms      2.36±0.03ms     0.66  arithmetic.MixedFrameWithSeriesAxis.time_frame_op_with_series_axis1('ne')
-      3.60±0.2ms      2.35±0.02ms     0.65  arithmetic.MixedFrameWithSeriesAxis.time_frame_op_with_series_axis1('lt')
-     47.8±0.08ms       28.4±0.1ms     0.59  frame_methods.Dropna.time_dropna_axis_mixed_dtypes('all', 1)
-      4.81±0.9ms      2.81±0.03ms     0.58  arithmetic.MixedFrameWithSeriesAxis.time_frame_op_with_series_axis1('truediv')
-     4.13±0.04ms       2.41±0.2ms     0.58  arithmetic.MixedFrameWithSeriesAxis.time_frame_op_with_series_axis1('eq')
-        11.9±1ms       6.88±0.5ms     0.58  arithmetic.MixedFrameWithSeriesAxis.time_frame_op_with_series_axis1('pow')
-     4.12±0.04ms      2.34±0.05ms     0.57  arithmetic.MixedFrameWithSeriesAxis.time_frame_op_with_series_axis1('le')
-        4.67±1ms       2.55±0.2ms     0.55  arithmetic.MixedFrameWithSeriesAxis.time_frame_op_with_series_axis1('mul')
-        4.90±1ms       2.55±0.2ms     0.52  arithmetic.MixedFrameWithSeriesAxis.time_frame_op_with_series_axis1('sub')
-      40.3±0.2ms       20.5±0.2ms     0.51  frame_methods.Dropna.time_dropna('all', 1)

Methodological note: i did a full asv run, then for everything that came back as changed re-ran those and changes that were not reproducible (e.g. tslibs benchmarks which have to be noise)

[TomAugspurger]

I'd like to move forward with this, including a version of the proposal on https://pandas.pydata.org/pandas-docs/dev/development/roadmap.html.

There hasn't been much activity on the google doc, https://docs.google.com/document/d/1csGE4qigPR2vzmU2--jwURn3sK5k5eVewinxd8OUPk0/edit. The biggest outstanding item is a decision / discussion of "Copy on Write" vs. "Error on Write". It'd be nice if the proposal was explicit on this point, but I worry we won't be able to decide without actually trying it out.

[jorisvandenbossche]

Some assorted thoughts I gathered the last weeks (not necessarily all needing an answer in an initial discussion / proposal, though):

• The copy/view in indexing discussion also impacts methods that rely on indexing internally (eg drop, dropna). I think that is also shown from the fact that dropna performance improves in the benchmark output above.
  So the indexing vs methods (what you put in the "extended proposal" section) are certainly somewhat intertwined, although it is of course possible to first only target indexing by ensuring we do additional copies internally in methods like drop that expect to return a copy.

• What to do with duplicate column selection? (df['a', 'a']]) Should that also still be a view? (if we would go for views) Mutating one column would then also mutate the other.

• What kind of mutation operations do we consider in this discussion? We clearly talked about "changing values in-place (setitem)", but there are also other kinds of mutation like changing metadata, flags, ..

• What to do with the constructors? (eg DataFrame(df)) Also copy-on-write? Or if we keep this as non-copy, what with constructors that have potentially a "reindexing" behaviour? (eg DataFrame(df, columns=[...]))

• What do we do with accessing a single column as a Series? Do we keep this as a pure view, or do we also want to (eventually) introduce copy-on-write/error for this? (after deprecation cycle)

• I think the crucial question that deserves more discussion is: are we fine with ditching the SettingWithCopyWarning for those cases it was introduced for?
  Because there is a reason that it was introduced: to warn uses that were doing a setitem operation that might have no effect silently, and thus be confusing / easy to miss for users.

  I think the classical example is something like df[df['B'] > 4]['B'] = 10 - a chained indexing assignment that doesn't work. The warning was introduced to prevent people doing that and silently having no effect.
  Are we fine with it that this will indeed have silently no effect? (which would be the consequence of copy-on-write)

[Dr-Irv]

I think the crucial question that deserves more discussion is: are we fine with ditching the SettingWithCopyWarning for those cases it was introduced for? Because there is a reason that it was introduced: to warn uses that were doing a setitem operation that might have no effect silently, and thus be confusing / easy to miss for users. I think the classical example is something like df[df['B'] > 4]['B'] = 10 - a chained indexing assignment that doesn't work. The warning was introduced to prevent people doing that and silently having no effect. Are we fine with it that this will indeed have silently no effect? (which would be the consequence of copy-on-write)

For me, this is one of the compelling arguments to go with "Error on Write". Then we are forcing people to be explicit about their intent, instead of depending on a behavior that is inconsistent (today) and for which we might not figure out all of the boundary cases if we chose "copy-on-write". Also, I've seen the following behavior with people I've worked with: 1) Ignoring the SettingWithCopyWarning, passing that code on to someone else who then has to deal with it 2) Writing a function that works perfectly fine in test scenarios with data read from a CSV file, but then handing off that function to someone else to use, who passes a different DataFrame that was constructed via some filtering operation, which then causes the warning to be issued. 3) Writing some code that is then passed to a totally different team, which treats all warnings as errors, in which case the code has to be fixed If we silently do copy-on-write, I'm concerned that these types of issues wouldn't get found by people using pandas. - Dr. Irv

[TomAugspurger]

Agreed that the classic chained indexing example is a compelling argument for Error on Write rather than Copy on Write.

What to do with duplicate column selection? (df[['a', 'a']]) Should that also still be a view? (if we would go for views) Mutating one column would then also mutate the other.

IMO, yes that should create two views to the same data.

What kind of mutation operations do we consider in this discussion? We clearly talked about "changing values in-place (setitem)", but there are also other kinds of mutation like changing metadata, flags, ..

I'd like to keep this focused just on setting data / values. I'm hopeful that the metadata / flags issue can be solved by doing shallow copies by default and providing an option to copy the data if desired, like we did for set_flags.

What do we do with accessing a single column as a Series?

If possible, treating it the same as a DataFrame seems best to me.

[jreback]

is there any system that does error-on-write? I do prefer this, but am a little concerned that we are treading new group vs. copy-on-write (which I think has a large body of systems / literature).

[jorisvandenbossche]

Putting it here because most of the discussion on this topic is still here (but can also move to #36988).

While the classic chained indexing example is indeed an argument for Error-on-Write (for this case, it's basically what we have now but with an error instead of warning), I think there are also arguments/examples to give in favor of Copy-on-Write:

• I would personally love to get the (IMO more common case) of subset = df[mask] or subset[list_of_columns] and then continuing with subset without worrying about the original df working out of the box, without the user needing to do .copy()/.copy_if_needed() (as is the case now if you don't want to risk getting the warning, even though those are always already copies)

• There are some cases where we currently don't raise a warning (eg adding a new column to a subset created like above), that woud start raising an error.

• We basically need Copy-on-Write anyway, because I think this still is the behaviour we want for methods, even if we do Error-on-Write for indexing? (drop, reindex, ...)

Also, I've seen the following behavior with people I've
worked with: ...

In this specific case, having consistent Copy-on-Write could also "solve" it? If is provides consistent behaviour (always copy, never mutate the passed data), not depending on the actual data passed to the function.
(of course, it depends on a lot on the actual case, and you were only giving a generic flow. But making things consistent in itself (always copy-on-write or always error-on-write) could also help avoiding such issues, I think? (while now you can actually mutate or not, when you get (and ignore) warning)

[jorisvandenbossche]

Assume that we could actually detect chained indexing? In this hypothetical situation, would people then be more in favor of Copy-on-Write over Error-on-Write?

The assumption would be that we can detect the difference in __setitem__ between

df[df['B'] > 4]['B'] = 10

vs

temp = df[df['B'] > 4]
temp['B'] = 10

I am wondering if we shouldn't be able to actually detect this based on reference counting. In the first case, there is no additional reference to df[df['B'] > 4] than the calling dataframe (self) of the second __setitem__ operation.
(I am not sure how robust such a detection would be though)

[Dr-Irv]

In this specific case, having consistent Copy-on-Write could also "solve" it? If is provides consistent behaviour (always copy, never mutate the passed data), not depending on the actual data passed to the function.
(of course, it depends on a lot on the actual case, and you were only giving a generic flow. But making things consistent in itself (always copy-on-write or always error-on-write) could also help avoiding such issues, I think? (while now you can actually mutate or not, when you get (and ignore) warning)

It's possible a consistent copy-on-write would solve this issue, but we have to make sure we get all of the cases right, and I'm just not convinced we'll be able to think all of those cases through. The other issue for me is that copy-on-write silently does something, which might have side effects that a user would like to be made aware of, whereas error-on-write makes the user very aware of what he is doing.

[jorisvandenbossche]

It's possible a consistent copy-on-write would solve this issue, but we have to make sure we get all of the cases right, and I'm just not convinced we'll be able to think all of those cases through.

Isn't the same true for Error-on-Write? We also need to detect all cases where we need to error instead of allow the mutation.

[jreback]

@jorisvandenbossche you have just described the current implementation of SettingCopyWarning and it's entire purpose

it is tracking refs

but i don't actually think this is that robust - and thats the same reason i am not enthusiastic about copy on write

yes if we can guarantee that we can always do it great

but that's the problem i am not sure we really can

though maybe now that we have full control over PandasArrays it might be fully possible (way back when when i did SettingWithCopy) some ops just indexes directly to the numpy arrays and we had to be very defensive about this

today we have much more robust handling