Skip to content
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

Fixed regression for SettingWithCopyWarning showing incorrect stacklevel #42633

Merged
merged 2 commits into from
Jul 22, 2021

Conversation

phofl
Copy link
Member

@phofl phofl commented Jul 20, 2021

Is there a better way to check the stacklevel here?

@phofl phofl added this to the 1.3.1 milestone Jul 20, 2021
@phofl phofl added Regression Functionality that used to work in a prior pandas version Copy / view semantics Indexing Related to indexing on series/frames, not to indexes themselves labels Jul 20, 2021
df = DataFrame(np.arange(25).reshape(5, 5))
chained = df.loc[:3]
with option_context("chained_assignment", "warn"):
with tm.assert_produces_warning(com.SettingWithCopyWarning) as t:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think check_stacklevel is supposed to do this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah thought so too, but test was passing with stacklevel 4 and 5

@@ -3745,7 +3745,7 @@ def _set_item_mgr(self, key, value: ArrayLike) -> None:
# try to set first as we want an invalid
# value exception to occur first
if len(self):
self._check_setitem_copy()
self._check_setitem_copy(stacklevel=5)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can use the find_stack_level function i think its called

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yah at first i was trying to use find_stack_level sparingly, but might as well go for it

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do that as a follow up, since this would change the function signature I don't think we should backport this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure that's fine

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@phofl
Copy link
Member Author

phofl commented Jul 22, 2021

merging

@phofl phofl merged commit 1f135eb into pandas-dev:master Jul 22, 2021
@phofl phofl deleted the 42570 branch July 22, 2021 22:54
meeseeksmachine pushed a commit to meeseeksmachine/pandas that referenced this pull request Jul 22, 2021
jreback pushed a commit that referenced this pull request Jul 23, 2021
…ng incorrect stacklevel (#42678)

Co-authored-by: Patrick Hoefler <[email protected]>
CGe0516 pushed a commit to CGe0516/pandas that referenced this pull request Jul 29, 2021
feefladder pushed a commit to feefladder/pandas that referenced this pull request Sep 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Copy / view semantics Indexing Related to indexing on series/frames, not to indexes themselves Regression Functionality that used to work in a prior pandas version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: Less helpful SettingWithCopyWarning on new pandas version
3 participants