ENH: infer_objects copy kwd by jbrockmendel · Pull Request #50096 · pandas-dev/pandas (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation5 Commits2 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
- closes #xxxx (Replace xxxx with the GitHub issue number)
- Tests added and passed if fixing a bug or adding a new feature
- All code checks passed.
- Added type annotations to new arguments/methods/functions.
- Added an entry in the latest
doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.
Comment on lines +377 to +392
| def convert(self: T, copy: bool) -> T: |
|---|
| def _convert(arr): |
| if is_object_dtype(arr.dtype): |
| # extract PandasArray for tests that patch PandasArray._typ |
| arr = np.asarray(arr) |
| return lib.maybe_convert_objects( |
| result = lib.maybe_convert_objects( |
| arr, |
| convert_datetime=True, |
| convert_timedelta=True, |
| convert_period=True, |
| ) |
| if result is arr and copy: |
| return arr.copy() |
| return result |
| else: |
| return arr.copy() |
| return arr.copy() if copy else arr |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a test that hits this?
TestInferObjects.test_copy should hit this in the CI build specific to ArrayManager
the plan is to "#48880 (comment)" anyway
I'd like this, but have no plans to push for it.
@jbrockmendel for 1.5 we had a discussion related to new copy keywords (#48141) and ended up reverting some of those new copy keywords. Is there a specific reason to still add a copy keyword to infer_objects?
Two big motivations. One was that we removed automatic inference in a few places, telling users to use infer_objects instead. Wanted to allow that operation to be cheap where possible. Second is that there was a bug in here in Block[Subclass].convert that was inconsistent about making copies. Fixing that bug required making more copies, and it's worth softening that blow.