Allow long numbers in numpy.rec.array formats string #376
Conversation
|
Can you elaborate on why this is a bug in numpy ("bug: it should accept Python's weird integer repr formats") instead of a bug in pyfits ("bug: it should use standard integer formats")? (Presumably the fix in pyfits would be to just use %s instead of %r somewhere. str() on integers produces standard format, repr() can do odd things like this.) |
|
It is common to end up with long numbers in strings: Or, probably more relevant, on win-amd64-py2.x: Besides that, I find it surprising that |
|
Just noticed that |
|
This looks good to me. Can somebody with push access merge this? |
|
I really think that as a general principle, we should be encouraging people to use the nice clean Python-data-structure ways of specifying dtypes, instead of adding more elaborate hacks to the domain-specific language parser. The bug here is that PyFITS has some nice Python representation of what it wants, but then instead of just giving us that representation it tries to serialize it into some string form, and its serialization code is, of course, buggy. This is a bad idea in exactly the same way that constructing source code by string manipulation for eval() is a bad idea. That said, we might as well merge this, given that np.rec.array is itself a grungy old interface that can't be adapted to handle anything except the bad old string-style formats= argument. (Can we deprecate formats=?) |
|
The way I see it, is that if (and this is the big if) we have a code like this: format_re = re.compile(asbytes(
r'(?P<order1>[<>|=]?)'
r'(?P<repeats> *[(]?[ ,0-9]*[)]? *)'
r'(?P<order2>[<>|=]?)'
r'(?P<dtype>[A-Za-z0-9.]*(?:\[[a-zA-Z0-9,.]+\])?)'))then we can just as well have a code like this: format_re = re.compile(asbytes(
r'(?P<order1>[<>|=]?)'
r'(?P<repeats> *[(]?[ ,0-9L]*[)]? *)'
r'(?P<order2>[<>|=]?)'
r'(?P<dtype>[A-Za-z0-9.]*(?:\[[a-zA-Z0-9,.]+\])?)'))Nothing has changed in terms of maintenance, simplicity, clarity or speed. But a bug was fixed. However, we can of course discuss whether we should have such a code in the first place, but that's a separate issue as I see it. |
|
@njsmith, let me know if you are ok with merging this PR. |
|
I already said I was :-) ("That said, we might as well merge this...") |
Allow long numbers in numpy.rec.array formats string
feat: Add vclt[q|s|d]_[s64|u64|f32|f64]
On win-amd64, some pyfits 3.0.9 tests fail because the
np.rec.arrayformatsargument may contain Python long numbers. A reduced example is:Tested with numpy 1.6.2 on win-amd64-py2.7 and win32-py2.7. Will work on a unit test later...