search

jjhelmus / nmrglue

A module for working with NMR data in Python
BSD 3-Clause "New" or "Revised" License
211 stars 86 forks source link

Trying to convert Bruker 'True' or 'False' values to floats. #26

Closed edward-dauvergne closed 9 years ago

edward-dauvergne commented 9 years ago

In the nmrglue/fileio/bruker.py file, the following code seems to be buggy:

""" def parse_jcamp_value(text): """ Parse value text from Bruker JCAMP-DX file returning the value. """ if "<" in text: return text[1:-1] # remove < and > elif "." in text or "e" in text or 'inf' in text: return float(text) else: return int(text) """

From running the full test suite with print statements for the 'text' argument, I can see that sometimes 'True' or 'False' strings are passed into this function. However with the "'e' in text'" check, this function identifies this as a float. This causes a ValueError. However the calling function read_jcamp(), via parse_jcamp_line(), has the dangerous catch all error "except:" statement. So this bad conversion is lost and a warning that the line cannot be parsed is given. An example line from the test data which fails here is:

$LOCSHFT= False

A solution would be to search for the text 'True' or 'False' in an elif statement just before this, and then use "return bool(text)".

jjhelmus commented 9 years ago

"True" and "False" are not valid Bruker JCAMP-DX text, the correct values for booleans are "no" and "yes". What process is producing the "##$LOCSHFT= False" line? This line is not present in the test data.

edward-dauvergne commented 9 years ago

Hi,

I'll have to look into it on Monday. It was a warning message from the nmrglue test suit, so maybe it is an auto-generated Bruker procpar file. It was not reproducible, so I have to see if it is a Python 3 vs. 2 issue or if it is specific to my fork and the 2to3 official Python conversion script changes (which would be rather strange). Do you have any multi-Python compatibility code in nmrglue?

Regards,

Edward

On Friday, 5 December 2014, Jonathan J. Helmus notifications@github.com wrote:

"True" and "False" are not valid Bruker JCAMP-DX text, the correct values for booleans are "no" and "yes". What process is producing the "##$LOCSHFT= False" line? This line is not present in the test data.

— Reply to this email directly or view it on GitHub https://github.com/jjhelmus/nmrglue/issues/26#issuecomment-65846119.

jjhelmus commented 9 years ago

The idiomatic change that is made by 2to3 is not an equivalent statement:

type(v) == int

is False when v is a boolean where as

isinstance(v, int)

is True when v is a boolean since bool is a subclass of int.

This causes the behaviour of the write_jcamp function to change when 2to3 is used to covert the bruker.py module.