-
-
Notifications
You must be signed in to change notification settings - Fork 18.2k
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
DEPR: Remove SettingWithCopyWarning #56614
Conversation
wow this kills giant hacks that i wrote from a while back! +1000000 |
"A value is trying to be set on a copy of a slice from a " | ||
"DataFrame\n\n" | ||
"See the caveats in the documentation: " | ||
"https://pandas.pydata.org/pandas-docs/stable/user_guide/" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are these docs still there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I’ll continue this pr later or
Tomorrow, the code things should be gone, docs not yet
# Conflicts: # pandas/tests/copy_view/test_indexing.py # pandas/tests/copy_view/test_internals.py # pandas/tests/copy_view/test_methods.py # pandas/tests/frame/indexing/test_xs.py # pandas/tests/indexing/multiindex/test_chaining_and_caching.py # pandas/tests/indexing/multiindex/test_setitem.py # pandas/tests/indexing/test_chaining_and_caching.py
This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this. |
# Conflicts: # ci/code_checks.sh # pandas/core/generic.py # pandas/core/groupby/groupby.py # pandas/errors/__init__.py # pandas/tests/copy_view/test_chained_assignment_deprecation.py # pandas/tests/copy_view/test_indexing.py # pandas/tests/indexing/test_chaining_and_caching.py # pandas/tests/series/test_ufunc.py
@@ -402,6 +395,7 @@ slicers on a single axis. | |||
Furthermore, you can *set* the values using the following methods. | |||
|
|||
.. ipython:: python | |||
:okwarning: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the pdep warning? Best to just change this example or remove if it won't work anymore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doesn't seem to be necessary anymore, lets see what ci has to say
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll keep this in for now, I can't reproduce locally but it shows up on ci, it's weird
doc/source/user_guide/indexing.rst
Outdated
reported. | ||
:ref:`Copy-on-Write <copy_on_write>` is the new default with pandas 3.0. | ||
This means than chained indexing will never work. As a consequence, | ||
the ``SettingWithCopyWarning`` is not necessary anymore. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would remove the As a consequence, the ``SettingWithCopyWarning`` is not necessary anymore.
sentence since that warning will no longer exist
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
pandas/core/frame.py
Outdated
@@ -4136,7 +4121,7 @@ def _get_value(self, index, col, takeable: bool = False) -> Scalar: | |||
series = self._ixs(col, axis=1) | |||
return series._values[index] | |||
|
|||
series = self._get_item_cache(col) | |||
series = self[col] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self._get_item(col)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hm yeah this should work, thx
pandas/core/generic.py
Outdated
if len(self._mgr.blocks) != blocks_before: | ||
self._clear_item_cache() | ||
return result | ||
return f() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we don't need _protect_consolidate
anymore right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point, no
Could you see if this works? |
@@ -58,8 +58,6 @@ Exceptions and warnings | |||
errors.PossiblePrecisionLoss | |||
errors.PyperclipException | |||
errors.PyperclipWindowsException | |||
errors.SettingWithCopyError |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you have another PR with the whatsnew note, please make sure that the removal of these are mentioned
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll create a separate PR with a whatsnew
# Conflicts: # doc/source/user_guide/indexing.rst
It works if you use it on a series but it will never work if you do df["a"].foo = "something because both df["a"] are different objects with CoW, so this can't work We can close the issue though, the case doesn't make sense under CoW |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel free to merge once you fix the merge conflict
# Conflicts: # pandas/_config/__init__.py # pandas/core/frame.py # pandas/core/generic.py # pandas/core/series.py # pandas/tests/copy_view/test_chained_assignment_deprecation.py # pandas/tests/copy_view/test_indexing.py # pandas/tests/copy_view/test_interp_fillna.py # pandas/tests/copy_view/test_methods.py # pandas/tests/copy_view/test_replace.py # pandas/tests/frame/indexing/test_indexing.py # pandas/tests/frame/indexing/test_xs.py # pandas/tests/indexing/multiindex/test_chaining_and_caching.py # pandas/tests/indexing/multiindex/test_setitem.py # pandas/tests/indexing/test_chaining_and_caching.py # pandas/tests/series/accessors/test_dt_accessor.py
This PR closed an issue I subscribed to (#9767), and I had no idea why and how until I found https://pandas.pydata.org/pdeps/0007-copy-on-write.html. Leaving this here for others. |
* DEPR: Remove SettingWithCopyWarning * Fixup * Remove docs * CoW: Boolean indexer in MultiIndex raising read-only error * Update * Update * Update
🥳
We shouldn't merge before the actual release is out
_item_cache
removalcloses #50547
closes #29411
closes #21391
SettingWIthCopyWarning removal
closes #18752
closes #9767
closes #14150
closes #16550
closes #17505
closes #38270
closes #39418
closes #39448
closes #41891
closes #45513
closes #50209
closes #55451
Others Matt thinks
closes #19102