Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix fibermap dtype for astropy 1.3 #380

Merged
merged 1 commit into from
Apr 26, 2017
Merged

fix fibermap dtype for astropy 1.3 #380

merged 1 commit into from
Apr 26, 2017

Conversation

sbailey
Copy link
Contributor

@sbailey sbailey commented Apr 24, 2017

This PR works around a change in behavior from astropy 1.2 to 1.3, likely related to the units issue we had to work around desihub/specsim#62:

Under astropy 1.2, assigning a value to a table column would retain the original dtype of that column. Under astropy 1.3, the column gets the new dtype of the right-hand-side value. This breaks some consistency checks for fibermap columns though I don't think it broke any actual functionality (for desispec).

This PR also fixes two smaller issues that I found while debugging this:

  • previously desispec.io.Brick couldn't be created with a filename that didn't include a path (it would parse the path as '' and then try to create '' and complain)
  • typo in new fibermap columns LOCATION,DEVICE_LOC,PETAL_LOC #379 (new fibermap columns) resulted in both LAMBDAREF and LAMBDA_REF

I have tested with astropy 1.3 on edison and astropy 1.2 on my laptop. I'm Leaving .travis.yml at astropy 1.2.1 for now; the nightly cronjob unit tests on edison test master with astropy 1.3.

@sbailey sbailey requested a review from dkirkby April 24, 2017 06:03
@sbailey
Copy link
Contributor Author

sbailey commented Apr 26, 2017

I'm going to go ahead and merge this now to get tests passing again under astropy 1.3 at NERSC, but feel free to make retroactive comments if you find something wrong.

@sbailey sbailey merged commit d7b5ad5 into master Apr 26, 2017
@sbailey sbailey deleted the brickdtype branch April 26, 2017 00:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant