Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ENH: Use lazy copy in infer objects #50428
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
ENH: Use lazy copy in infer objects #50428
Changes from 26 commits
3c4cee3
db5dba1
9eb4d85
081dd29
76c443b
f7724ff
a7b4e27
1d4f726
018cfe6
1696d8a
1782fbd
8cb6355
cead228
47d85b3
a3d0a2b
2e2ed0f
3a84382
64d550b
716cef8
f693829
97fa214
c3e4e66
5d687dd
f5bf65f
af8af95
f47f6dd
e357b64
bf1bb3b
5624983
9a6d516
8b0e2b0
3f832ba
9f11f0a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
small leftover from the merge
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.
Yep, thx, I am already going over them. There seem to be a couple of things off
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.
Did something change here, or also a merge leftover?
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.
Same yes, Updated everything now, should be good to go.
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.
It might be worth adding a test where one of the object dtype columns actually gets converted? (eg change d to
"d": np.array([0, 1], dtype=object)
) To ensure this column owns its memory (and cover that part of the code additions)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, adjusted d in the two tests below to cover both cases.
Good to go apart from that?
convert
needs cow for a couple of other things