Adding floating hex (ala float.fromhex) support to loadtxt converter. (Issue 1924) #133

Closed
wants to merge 1 commit into
from

Projects

None yet

4 participants

@claumann
Contributor
claumann commented Aug 4, 2011

Add _floatconv to npyio.py as a default floating point converter. This
uses float() as a type conversion with a fallback on (ValueError) to
float.fromhex().

I've never submitted a github style patch to a project before. I hope I'm not jumping the gun by submitting a pull request for a feature without any discussion after my feature request #1924. What's the appropriate etiquette?

@claumann claumann Added support for float hex format to loadtxt.
Add _floatconv to npyio.py as a default floating point converter. This
uses float() as a type conversion with a fallback on (ValueError) to
float.fromhex().
81ee356
@rgommers
Member

You can submit a pull request directly, but in case of a new feature it's usually better to discuss the idea on-list first. Tickets on Trac often take a long time before someone looks at them unfortunately.

A few remarks about your code:

  • it needs unit tests before it can be accepted
  • checking for '0x' as you suggest on #1924 would be better than trying the hex conversion by default
  • can the same thing be achieved by using a converter (see converters keyword)?
  • can you time the performance hit for a file with normal floats? since this converter is called very often it may not be negligible.
  • why is the change to .gitignore necessary? .DS_Store? works for me.
@mwiebe
Member
mwiebe commented Aug 19, 2011

Looks like float.fromhex is new in Python 2.6, how does this behave in 2.4/2.5?

@charris
Member
charris commented Sep 17, 2011

I can confirm that float.fromhex is missing from Python 2.5. So unless the implementation can be changed this addition will have to wait. The Python code for this is in C which I suppose we could add to _compiled_base.c, but unless someone wants this feature badly and is willing to port the C code, I don't think it is worth the hassle..

I suppose we could also make the feature depend on the Python version.

@charris
Member
charris commented Feb 20, 2014

Now that 2.4 and 2.5 have been dropped, we can take a second look at this. @claumann Want to rebase this?

@charris
Member
charris commented Jan 25, 2015

Can revisit this now, for numpy >= 1.8 only python >= 2.6 is supported.

@charris
Member
charris commented Jan 25, 2015

Cleaned up in #5504.

@charris charris closed this Jan 25, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment