BUG: Various fixes to _dtype_from_pep3118 #9054
Conversation
Adding padding fields means that dtypes with the same fields cannot necessarily be cast to one another. Padding shouldn't prevent casting. Partially fixes numpy#9053
|
I like the solution with the stream class (though I wonder to what extent you're reinventing |
|
p.s. would be nice to have #8774 so you don't have to redefine |
|
#8774 actually falls back on |
I've seen in the past that dependencies for numpy are a pain from a distribution perspective (#that-pr-about-deprecating-np.float), and it's better to do simple things ourselves. |
|
Yes, makes sense. And as said, I quite like your stream class; and the path that gets taken is at least a bit clearer than with a lex file (or a regex). |
I read over it well enough that it has my +1 for merging. If there are no other comments I would be happy to merge in a day or two. |
|
All right, pulling the trigger here. Thanks @eric-wieser ! |
Can you give a reference for that? |
|
Is it that clear? As I understand, the padding is supposed to follow
typical C compiler behavior.
|
|
Yes, but there is also a question on whether struct.Struct
implementation is correct.
|
|
Since also `struct.Struct('ib').size == 5`, I guess the question then is
whether trailing padding must be explicit or not. The PEP does not spell
this out. Python's struct module syntax is older than the PEP, so maybe
that is the behavior that has priority.
|
|
Here's the reference, (copied from my comment in #7798): The struct module says this about how format strings treat trailing padding:
My interpretation is that for a format like 'ix', even though the type is implicitly |
Fixes #9053, and groundwork for #9049.
Again, probably easier to review commit-by-commit