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.

Sign up for GitHub

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

Jump to bottom

DEPR: Deprecate is_copy (#18801) #18812

Merged
merged 3 commits into from
Dec 21, 2017
Merged

DEPR: Deprecate is_copy (#18801) #18812

merged 3 commits into from
Dec 21, 2017

Conversation

cbertinato
Copy link
Contributor

@cbertinato cbertinato commented Dec 17, 2017 •
edited by gfyoung
Loading

Not all tests for this branch on the fork are passing, but these are the same tests that fail for the clean master and appear to be unrelated to the changes.

References to is_copy in tests were changed to the internal attribute _is_copy assuming that, though is_copy is not meant to be a public attribute, checking the state of this attribute is necessary to check consistent internal behavior.

Closes #18801.

@codecov
Copy link

codecov bot commented Dec 17, 2017 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (master@b2a02bd). Click here to learn what that means.
The diff coverage is 95.83%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #18812   +/-   ##
=========================================
  Coverage          ?   91.62%           
=========================================
  Files             ?      154           
  Lines             ?    51407           
  Branches          ?        0           
=========================================
  Hits              ?    47103           
  Misses            ?     4304           
  Partials          ?        0
Flag Coverage Δ
#multiple 89.49% <95.83%> (?)
#single 40.84% <41.66%> (?)
Impacted Files Coverage Δ
pandas/core/indexes/accessors.py 89.36% <100%> (ø)
pandas/core/indexing.py 92.81% <100%> (ø)
pandas/core/generic.py 95.92% <95.23%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b2a02bd...099cd33. Read the comment docs.

@gfyoung gfyoung added the Deprecate Functionality to remove in pandas label Dec 17, 2017
gfyoung
gfyoung reviewed Dec 17, 2017
View reviewed changes
pandas/core/generic.py Outdated
@property
def is_copy(self):
warnings.warn("Attribute 'is_copy' will be removed in a future "
"version.", FutureWarning, stacklevel=2)
Copy link
Member

Choose a reason for hiding this comment

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

Say : "is deprecated and will removed..."

Copy link
Member

Choose a reason for hiding this comment

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

Also, add a test for this deprecation (both for getter and setter).

@jsexauer jsexauer mentioned this pull request Dec 18, 2017
Closed
1 task
jreback
jreback requested changes Dec 18, 2017
View reviewed changes
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.

can you add a whatsnew note in the deprecation section & a test (as @gfyoung indicates); you can put this in pandas/tests/indexing/test_chainging_and_caching.py

DEPR: Deprecate is_copy (pandas-dev#18801)
9b8f2af 
- Renamed 'is_copy' attribute to '_is_copy' for internal use
- Setup getter and setter for 'is_copy'
- Added tests for deprecation warning
@pep8speaks
Copy link

pep8speaks commented Dec 18, 2017 •
edited
Loading

Hello @cbertinato! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on December 19, 2017 at 12:20 Hours UTC

@cbertinato
Copy link
Contributor Author

Shall I squash these commits into one and resubmit, or can we do it on the merge?

@jreback
Copy link
Contributor

jreback commented Dec 18, 2017

no need to squash

will be auto done on merge

@cbertinato
Copy link
Contributor Author

I am by no means in a rush, but just wanted to check whether this is waiting on anything else. I believe that the requested changes have been made.

@gfyoung
Copy link
Member

gfyoung commented Dec 20, 2017

@cbertinato : There's nothing for you to do ATM, but we're currently having build issues on Travis for our master branch. @jreback is trying to resolve those first before we merge anything else.

@cbertinato
Copy link
Contributor Author

Roger that!

@jreback jreback added this to the 0.23.0 milestone Dec 21, 2017
@jreback jreback merged commit 6d2fb3e into pandas-dev:master Dec 21, 2017
@jreback
Copy link
Contributor

jreback commented Dec 21, 2017

thanks @cbertinato

@cbertinato cbertinato deleted the issue-18801 branch December 22, 2017 13:14
@topper-123
Copy link
Contributor

When tab-completing in iPython, the FutureWarning for is_copy pops up.

I believe that 'is_copy' should be added to _deprecations

@jreback
Copy link
Contributor

jreback commented Dec 23, 2017

yep that is right - PR?

@cbertinato
Copy link
Contributor Author

PR #18922

@jreback jreback mentioned this pull request Nov 21, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gfyoung gfyoung gfyoung left review comments

@jreback jreback jreback approved these changes

Assignees
No one assigned
Labels
Deprecate Functionality to remove in pandas
Projects
None yet
Milestone
0.23.0
Development

Successfully merging this pull request may close these issues.
5 participants
@cbertinato @pep8speaks @jreback @gfyoung @topper-123