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.
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
Add public APIs to Access Underlying
cudf
andpandas
Objects fromcudf.pandas
Proxy Objects #17629Add public APIs to Access Underlying
cudf
andpandas
Objects fromcudf.pandas
Proxy Objects #17629Changes from 18 commits
b5eea1f
7bc76e5
3cdfe94
34375dc
31f9e99
72ba73f
3fd679f
37764c2
bbe0fa4
cf6888f
528c189
6b744f4
9ec0215
a3c49fd
3f06e70
95ba799
a2e97f5
b96f8ff
0044d8f
a77fecc
aafbd31
f15d37b
f94253d
daaa5df
eaa23cb
9c58a71
69837f3
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.
Could we use
_mimic_inplace
instead?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 feel this is much light weight that using
_mimic_inplace
and also copies cached attributes.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.
Can we remove this?
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.
removed.
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 functions actually useful? It seems like it would be better to just tell the user how to check this themselves with
is_proxy_object
+isinstance
.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.
Yes, we could do that but my intention here was to just have single API that informs the users. It will end up being a verbose pattern in libraries that operate
cudf
/cudf.pandas
aware.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.
My suggestion in a thread above was to instead implement a single method (bikeshed the name as much as you wish, I'm mostly concerned with suggesting an implementation)
so that they don't need to be aware of
is_proxy_object
and it "feels like" they're usingisinstance
.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.
Does it make sense to call
isinstance_cudf_pandas
inside this function and raise a friendly error message on False?