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

Make NumpyArray's format checks insensitive to '=' (or '<' if little-endian, '>' if big-endian) at the beginning of the string #313

Closed
jpivarski opened this issue Jun 26, 2020 · 1 comment · Fixed by #337
Labels
bug The problem described is something that must be fixed good first issue Good for newcomers

Comments

@jpivarski
Copy link
Member

For minimal modification (and a tiny performance advantage), this can be done in the NumpyArray constructor. (Also check the NumpyForm and conversions from JSON into NumpyForms.) That way, the unnecessary character can be removed once in one place, without having to fix every instance in NumpyArray where format_ is used.

If NumPy arrays are created as big-endian and then are converted with astype to little-endian, they have this extra character in their formats. Uproot4 is using a work-around so that a new version of Awkward with this fix is not needed right away, but it's important to fix anyway.

This would be an easy fix, though it's entirely in C++, not Python.

@jpivarski jpivarski added bug The problem described is something that must be fixed good first issue Good for newcomers labels Jun 26, 2020
@jpivarski
Copy link
Member Author

Actually, fixing this bug means the following array copy in Uproot won't be necessary:

https://github.com/scikit-hep/uproot4/blob/43306c11eb750e78247a44fd30a2b0a88b0c4449/uproot4/interpretation/library.py#L140-L141

This will happen again when dealing with fixed-width objects (i.e. TLorentzVector), so it's worth fixing and dealing with the Awkward1 ↔ Uproot4 version synchronization.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The problem described is something that must be fixed good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant