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

_dtype_from_pep3118 is overly strict on prefixes #9049

Open
bmerry opened this issue May 4, 2017 · 1 comment
Open

_dtype_from_pep3118 is overly strict on prefixes #9049

bmerry opened this issue May 4, 2017 · 1 comment

Comments

@bmerry
Copy link
Contributor

bmerry commented May 4, 2017

In numpy 1.12.1, _dtype_from_pep3118 requires that modifiers are specified in the order shape then byte order then count, with at most one of each. The PEP is a bit vague, but it seems like modifiers ought to be interpreted recursively i.e. a modifier can be followed by any valid PEP 3118 specifier.

In practice this can lead to certain types failing to round-trip through a memoryview e.g.:

>>> dt = np.dtype({'formats': [np.dtype((np.dtype((np.int32, (3,))), (2,)))], 'names': ['foo']})
>>> a = np.empty(0, dt)
>>> m = memoryview(a)
>>> np.array(m)
NotImplementedError: memoryview: unsupported format T{(2)(3)i:foo:}

It also causes problems if one puts an endianness specifier at the start of the string (which should always be permitted, since the original struct module allows it). I haven't managed to get numpy to generate such a format string when wrapping an array into a memoryview, but it's causing me some problems while developing new features for pybind11:

>>> np.core._internal._dtype_from_pep3118('<(3)i')
ValueError: Unknown PEP 3118 data type specifier '(3)i'
@eric-wieser
Copy link
Member

eric-wieser commented May 4, 2017

Seems odd to me that we implement _buffer_format_string in C, but _dtype_from_pep3118, the inverse, in python

bmerry added a commit to bmerry/pybind11 that referenced this issue May 4, 2017
This moves the ^ in the format string (to specify unaligned) to
outside the `T{}` where it is sure to be parsed correctly. This is not
strictly necessary yet, but it paves the way for pybind#832.
bmerry added a commit to bmerry/pybind11 that referenced this issue May 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants