search

desihub / desimodel

Code that reads and processes the desimodel data files.
BSD 3-Clause "New" or "Revised" License
3 stars 3 forks source link

Problem with new version of numpy #166

Closed echaussidon closed 10 months ago

echaussidon commented 1 year ago

Hi,

I'm using python3.10 with numpy = 1.24.3.

https://github.com/desihub/desimodel/blob/3f4bf408b48261c01e354ff3fd62dd16352eae01/py/desimodel/io.py#L377

does not raise 'IndexError' but 'ValueError'. So, I cannot use the code, as it is.

Instead of playing with the exception, I suggest to just try the shape of this file:

if np.loadtxt(infile).shape[1] == 8: DO else: DO

weaverba137 commented 1 year ago

@sbailey, when I look at the git history, it looks like you created that line, with commit f95b26b. It's not exactly clear at first glance what the most desirable outcome of this function is. In other words, do we have to continue supporting both methods of loading the data?

dstndstn commented 1 year ago

I just hit this error. I note that the current version of the file in desimodel-data (0.18.0 and trunk) has only 8 columns, so the way the code first tries to read the file will always fail, and it depends on getting an IndexError in order to do the right thing, while some versions of numpy instead raise a ValueError. So at the very least, the expected path should be the path that doesn't demand the right kind of exception being raised :)

dstndstn commented 1 year ago

Minimal change would be to make line 377

except (IndexError,ValueError):
sbailey commented 1 year ago

This dates back to a very early version of the file and supporting the transition period after the "arclength" column was dropped from the file provided by the engineering team. I'm fine with either updating the code to support only the current format, or updating to except (IndexError, ValueError): as the minimal change to restore functionality.

moustakas commented 10 months ago

Looks like this issue was addressed by @dstndstn in https://github.com/desihub/desimodel/commit/a98e1ef44fbb4a355e69b1b952b08950920e5703; closing.