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.

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

Test get_buffer_data and get_buffer_nbytes #549

Merged
merged 4 commits into from
Jun 16, 2020

Conversation

jakirkham
Copy link
Member

@jakirkham jakirkham commented Jun 16, 2020

Needed for PR ( #546 )

Adds some tests for the utility functions get_buffer_data and get_buffer_nbytes using both Python builtin objects and NumPy/CuPy arrays. Should exercise both the memoryview case as well as the __array_interface__/__cuda_array_interface__ case.

Provide some initial testing of the buffer utility functions with Python
builtin types. This provides us something pretty useful that we can run
through without having to worry about any dependencies to start with.
@jakirkham jakirkham requested a review from a team as a code owner June 16, 2020 01:03
@jakirkham
Copy link
Member Author

cc @quasiben @pentschev

@jakirkham jakirkham force-pushed the test_libs_utils branch 5 times, most recently from 54fcb02 to 06bbdfc Compare June 16, 2020 01:17
Include some tests using NumPy and CuPy arrays. Check that pointers are
what we expect them to be. Also check that `nbytes` is computed
correctly.
Copy link
Member

@pentschev pentschev left a comment

Choose a reason for hiding this comment

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

I have a couple of minor questions only, otherwise it looks good. Also, do these tests allow us to check for the issue from #546 (comment) ?

arr = xp.ndarray(shape, dtype, data.data, strides=strides)
iface = getattr(arr, iface_prop)

return xp, arr, iface
Copy link
Member

Choose a reason for hiding this comment

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

Seems like xp isn't currently used for anything. Just wanted to check this is intentional.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I wasn't sure if we would need it or not. So left in the option. We could drop it if you prefer.

Copy link
Member

Choose a reason for hiding this comment

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

No worries, just wanted to confirm.

tests/test_libs_utils.py Show resolved Hide resolved
Copy link
Member

@quasiben quasiben left a comment

Choose a reason for hiding this comment

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

LGTM

@pentschev
Copy link
Member

LGTM, thanks @jakirkham .

@pentschev pentschev merged commit 3e64dbb into rapidsai:branch-0.15 Jun 16, 2020
@jakirkham jakirkham deleted the test_libs_utils branch June 16, 2020 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants