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

Make proxy NumPy arrays pass isinstance check in cudf.pandas #16286

Merged
merged 25 commits into from
Aug 16, 2024
Merged

Make proxy NumPy arrays pass isinstance check in cudf.pandas #16286

merged 25 commits into from
Aug 16, 2024

Conversation

Matt711
Copy link
Contributor

@Matt711 Matt711 commented Jul 16, 2024

Description

Apart of #15397. Closes #14537. Creates ProxyNDarray which inherits from np.ndarray.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@Matt711 Matt711 added 5 - DO NOT MERGE Hold off on merging; see PR for details non-breaking Non-breaking change cudf.pandas Issues specific to cudf.pandas labels Jul 16, 2024
@Matt711 Matt711 added this to the Proxying - cudf.pandas milestone Jul 16, 2024
@Matt711 Matt711 self-assigned this Jul 16, 2024
@github-actions github-actions bot added the Python Affects Python cuDF API. label Jul 16, 2024
@Matt711 Matt711 added feature request New feature or request and removed 5 - DO NOT MERGE Hold off on merging; see PR for details labels Jul 16, 2024
@Matt711 Matt711 force-pushed the feature/numpy-proxying branch from 7491769 to 365e1b0 Compare July 16, 2024 16:20
@Matt711
Copy link
Contributor Author

Matt711 commented Jul 16, 2024 •
edited
Loading

The following numba on proxy numpy array works too. Closes #15694

In [1]: %load_ext cudf.pandas

In [2]: import pandas as pd

In [3]: from numba import njit

In [4]: @njit(nogil=True)
   ...: def f(x):
   ...:     return 2*x
   ...: 

In [5]: f(pd.Series([1]).values)
Out[5]: array([2])

Edit: There is fallback though

AttributeError. Falling back to the slow path. The exception was 'Flags' object has no attribute 'writeable'.

Matt711
Matt711 commented Jul 17, 2024
View reviewed changes
Copy link
Contributor Author

@Matt711 Matt711 left a comment

Choose a reason for hiding this comment

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

So this approach creates a Proxy{Insert NumPy/Slow Object Name}Base class (which inherits from the real NumPy/slow class) as a base class for the proxy type.

python/cudf/cudf/pandas/fast_slow_proxy.py Outdated Show resolved Hide resolved
python/cudf/cudf/pandas/fast_slow_proxy.py Outdated Show resolved Hide resolved
@copy-pr-bot copy-pr-bot
Copy link

copy-pr-bot bot commented Jul 19, 2024

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@galipremsagar
Copy link
Contributor

/okay to test

@Matt711 Matt711 changed the base branch from branch-24.08 to branch-24.10 July 24, 2024 15:39
@Matt711
Copy link
Contributor Author

Matt711 commented Jul 24, 2024

/ok to test

@Matt711
Copy link
Contributor Author

Matt711 commented Jul 30, 2024

/ok to test

@Matt711
Copy link
Contributor Author

Matt711 commented Aug 13, 2024

/ok to test

@Matt711 Matt711 marked this pull request as ready for review August 13, 2024 13:50
@Matt711 Matt711 requested a review from a team as a code owner August 13, 2024 13:50
@Matt711 Matt711 requested review from vyasr and galipremsagar August 13, 2024 13:50
@Matt711
Copy link
Contributor Author

Matt711 commented Aug 13, 2024

/ok to test

@galipremsagar
Copy link
Contributor

/okay to test

@galipremsagar
Copy link
Contributor

@mroeschke Could you take a look before we merge this PR?

@galipremsagar
Copy link
Contributor

/okay to test

@Matt711
Copy link
Contributor Author

Matt711 commented Aug 16, 2024

/ok to test

@Matt711
Copy link
Contributor Author

Matt711 commented Aug 16, 2024

/merge

@rapids-bot rapids-bot bot merged commit 1c63e1e into rapidsai:branch-24.10 Aug 16, 2024
73 checks passed
@Matt711 Matt711 changed the title Initial investigation into NumPy proxying in cudf.pandas Make proxy NumPy arrays pass isinstance check in cudf.pandas Aug 16, 2024
@Matt711 Matt711 mentioned this pull request Aug 16, 2024
Closed
Matt711 added a commit that referenced this pull request Aug 16, 2024
@Matt711
Revert "Initial investigation into NumPy proxying in cudf.pandas (#…
7fa8b79 
…16286)"

This reverts commit 1c63e1e.
@Matt711 Matt711 mentioned this pull request Aug 16, 2024
Merged
rapids-bot bot pushed a commit that referenced this pull request Aug 17, 2024
@Matt711
Revert "Make proxy NumPy arrays pass isinstance check in `cudf.pandas…
dd2c12d 
…`" (#16586)

Reverts #16286

Authors:
  - Matthew Murray (https://github.com/Matt711)
  - GALI PREM SAGAR (https://github.com/galipremsagar)

Approvers:
  - GALI PREM SAGAR (https://github.com/galipremsagar)

URL: #16586
@Matt711 Matt711 mentioned this pull request Aug 27, 2024
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mroeschke mroeschke mroeschke approved these changes

@galipremsagar galipremsagar galipremsagar approved these changes

@vyasr vyasr Awaiting requested review from vyasr vyasr is a code owner automatically assigned from rapidsai/cudf-python-codeowners

Assignees

@Matt711 Matt711

Labels
cudf.pandas Issues specific to cudf.pandas feature request New feature or request non-breaking Non-breaking change Python Affects Python cuDF API.
Projects
cuDF Python
Status: Done
Milestone
Proxying - cudf.pandas
Development

Successfully merging this pull request may close these issues.

[FEA] Proxy ndarrays don't pass an instancecheck() for np.ndarray
3 participants
@Matt711 @galipremsagar @mroeschke