Skip to content

Commit

Permalink
Apply suggestions from code review
Browse files Browse the repository at this point in the history
Co-authored-by: Irv Lustig <[email protected]>
  • Loading branch information
jorisvandenbossche and Dr-Irv authored Oct 6, 2023
1 parent 55e1685 commit a9879ce
Showing 1 changed file with 6 additions and 6 deletions.
12 changes: 6 additions & 6 deletions web/pandas/pdeps/0007-copy-on-write.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ returning views vs. copies.

We also care about performance. Returning views from indexing operations is faster and
reduces memory usage. The same is true for several methods that don't modify the data
such as setting/resetting the index, renaming columns, etc that can be used in a method
such as setting/resetting the index, renaming columns, etc. that can be used in a method
chaining workflow and currently return a new copy at each step.

Finally, there are API / usability issues around views. It can be challenging to know
Expand All @@ -65,9 +65,9 @@ There are two possible behaviours the user might intend:

Today, pandas' inconsistency means _neither_ of these workflows is really possible. The
first is difficult, because indexing operations often (though not always) return copies,
and even when a view is returned you sometimes get a SettingWithCopyWarning when
and even when a view is returned you sometimes get a `SettingWithCopyWarning` when
mutating. The second is somewhat possible, but requires many defensive copies (to avoid
SettingWithCopyWarnings, or to ensure that you have a copy when a view _was_ returned).
`SettingWithCopyWarning`, or to ensure that you have a copy when a view _was_ returned).

## Proposal

Expand Down Expand Up @@ -95,7 +95,7 @@ modified is a view on another dataframe (or is being viewed by another dataframe
is, then we would copy the data before mutating.

Taking the example from above, if the user wishes to not mutate the parent, we no longer
require a defensive copy just to avoid a SettingWithCopyWarning.
require a defensive copy just to avoid a `SettingWithCopyWarning`.

```python
# Case 2: The user does not want mutating df2 to mutate the parent df, via CoW
Expand Down Expand Up @@ -303,7 +303,7 @@ See the GitHub comment with some more detailed argumentation:
## Disadvantages

Other than the fact that this proposal would result in a backwards incompatible,
breaking change in behaviour (see next section), there some other potential
breaking change in behaviour (see next section), there are some other potential
disadvantages:

* Deviation from NumPy: NumPy uses the copy and view concepts, while in this proposal
Expand Down Expand Up @@ -403,7 +403,7 @@ cases will stop working (e.g. the case above but switching the order):
```

These cases will raise a warning ``ChainedAssignmentError``, because they can never
accomplished what the user intended. There will be false-positive cases when these
accomplish what the user intended. There will be false-positive cases when these
operations are triggered from Cython, because Cython uses a different reference counting
mechanism. These cases should be rare, since calling pandas code from Cython does not
have any performance benefits.
Expand Down

0 comments on commit a9879ce

Please sign in to comment.