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

test_read_fits_spec.py, the first two test cases failing #68

Closed
shaileshahuja opened this issue May 29, 2014 · 5 comments
Closed

test_read_fits_spec.py, the first two test cases failing #68

shaileshahuja opened this issue May 29, 2014 · 5 comments

Comments

@shaileshahuja
Copy link
Contributor

https://github.com/astropy/specutils/blob/master/specutils/io/tests/test_read_fits_spec.py#L20
iraf wave array:
[4344.5500522758402, 4344.59028429296, 4344.6305137707004, 4344.6707407088797, 4344.7109651072797]
fits dispersion array:
[4344.5098177195632, 4344.5500522758412, 4344.5902842929554, 4344.6305137707031, 4344.6707407088788]

The (n+1)th element in fits dispersion array is the same as the nth element in the iraf wave array.
@wkerzendorf @nhmc

@shaileshahuja
Copy link
Contributor Author

@wkerzendorf @nhmc this issue needs to be resolved before #69 can be merged. Is the problem with the IRAF data files, or the WCS functions?

@nhmc
Copy link

nhmc commented Jun 10, 2014

I believe the problem is with the WCS functions - they assign the first pixel a value of zero, (instead of one, which is what IRAF does). There's a keyword to the WCS method .wcs_pix2world() which accepts 0 or 1 to indicate the indexing convention. So I think that's all that needs to be changed.

@nhmc
Copy link

nhmc commented Jun 11, 2014

It looks like the arange at line 263 in https://github.com/astropy/specutils/blob/master/specutils/spectrum1d.py needs to be changed to accept a starting index of either 0 or 1. I would add a new keyword to Spectrum1D.__init__ to indicate either a 0 or 1 indexing convention. This would set an attribute to be checked in Spectrum1D.dispersion to see whether arange should start with 0 or 1.

@wkerzendorf, do you think this is the best approach?

@wkerzendorf
Copy link
Member

What @nhmc says, is very likely the cause of the problem. The indexing in IRAF starts with 1 (maybe because FORTRAN uses this idea). I think, however, that we should stay with 0-indexing throughout specutils. That will avoid any pitfalls. As a step forward we need to make sure that when we read-in WCS from fits files to subtract 1.

@shaileshahuja
Copy link
Contributor Author

This issue will be fixed with #72

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

No branches or pull requests

3 participants