-
-
Notifications
You must be signed in to change notification settings - Fork 129
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
Use the APE 14 specified API in some of the tests #1195
Conversation
…evel object conversion not a highlevel one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the clean up!
specutils/tests/test_spectrum1d.py
Outdated
@@ -318,15 +318,15 @@ def test_wcs_transformations(): | |||
spec = Spectrum1D(spectral_axis=np.arange(1, 50) * u.nm, | |||
flux=np.ones(49) * u.Jy) | |||
|
|||
pix_axis = spec.wcs.world_to_pixel(np.arange(20, 30) * u.nm) | |||
pix_axis = spec.wcs.world_to_pixel_values(np.arange(20, 30) * u.nm) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FAILED ../../specutils/tests/test_spectrum1d.py::test_wcs_transformations - AttributeError: 'numpy.ndarray' object has no attribute 'to_value'
I don't think this patch is correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I'll have to adjust things. It's because units have been passed in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
spacetelescope/gwcs#457 also corrects things so that the *_values
can accept quantity inputs. GWCS was inconsistent about this. For now its better to just wrap it in a SpectralCoord
object like it should be for the function as it is used in the test.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1195 +/- ##
=======================================
Coverage 86.92% 86.92%
=======================================
Files 63 63
Lines 4572 4572
=======================================
Hits 3974 3974
Misses 598 598 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
Remote data failure is unrelated. |
pix_axis = spec.wcs.world_to_pixel(np.arange(20, 30) * u.nm) | ||
# After spacetelescope/gwcs#457 is merged and released, this can be changed to | ||
# pix_axis = spec.wcs.world_to_pixel_values(np.arange(20, 30) * u.nm) | ||
pix_axis = spec.wcs.world_to_pixel(SpectralCoord(np.arange(20, 30) * u.nm)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fine. I think it makes sense for specutils to use high level API.
spacetelescope/gwcs#457 corrects an oversight in the APE 14 related API, where "low-level" inputs were accepted by the "high-level" api. This PR simply swaps the usage in specutils from using the "high-level" with "low-level" inputs to using the proper "low-level" API.
Also, I updated the tests so that files generated by the tests get cleaned up by pytest.