-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
CLN: Use more memoryviews #58330
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
CLN: Use more memoryviews #58330
Conversation
@@ -80,7 +80,7 @@ def unstack(numeric_object_t[:, :] values, const uint8_t[:] mask, | |||
|
|||
@cython.wraparound(False) | |||
@cython.boundscheck(False) | |||
def explode(ndarray[object] values): |
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 get here with a readonly ndarray?
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.
Looks like in our testing this array is always writable (from mgr.internal_values()
), should this be readonly?
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 think it can depend on what user passed to the constructor
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.
Gotcha. Well specifically here, we can't use const
as that's not supported with object memoryviews
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.
lgtm
* Add memoryviews in reshape.pyx * Use more const memoryviews
* Add memoryviews in reshape.pyx * Use more const memoryviews
No description provided.