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

Expose a few ArrayDescriptor properties and option for mutable flags #4451

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

fwilliams
Copy link

Description

This change exposes a few elements in the PyArrayDescr struct which I am using in another project.

It also exposes a mutable version of flags() in pybind11::array(), which is useful when you need to steal the memory from a numpy array accross python calls.

#if defined(_MSC_VER)
# pragma warning(push)
# pragma warning(disable : 4127) // warning C4127: Conditional expression is constant
typedef SSIZE_T ssize_t;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Huh, why is this typedef necessary?

@@ -18,6 +18,7 @@
#include <cstdlib>
#include <cstring>
#include <functional>
#include <iostream>
Copy link
Collaborator

Choose a reason for hiding this comment

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

no need for iostream include

@fwilliams
Copy link
Author

whoops, had some old code remaining from my fork that I forgot to remove. Fixed this.

@Skylion007 Skylion007 requested a review from rwgk January 11, 2023 22:23
@rwgk
Copy link
Collaborator

rwgk commented Jan 11, 2023

@fwilliams could you please add tests for the new methods?

Especially the mutable flags method: I'm not very familiar with numpy, do we have someone with more background than I have? @henryiii, @ax3l?

I had a quick look here:

https://numpy.org/doc/stable/reference/generated/numpy.ndarray.flags.html

What stands out to me:

or by calling ndarray.setflags
...
The array flags cannot be set arbitrarily:

My gut feeling is that it would be safer not to expose the direct mutable reference, but to add setflags support.

which is useful when you need to steal the memory from a numpy array accross python calls

That sounds very dangerous (bug prone) and would need careful explanation and unit testing.

@Skylion007
Copy link
Collaborator

I agree with @rwgk about it's probably better to use set_flags method. @rwgk we may want to update our code to also set the flags this way when possible.

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.

None yet

3 participants