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

Fixed return type and shape for pixel <-> world conversions in SpectralCoordinates #82

Merged
merged 3 commits into from
Jan 19, 2023

Conversation

astrofrog
Copy link
Member

The pixel <-> world methods should return single scalars/arrays not tuples in order to be APE-14-compliant.

@pllim
Copy link
Contributor

pllim commented Jan 19, 2023

Is it a concern that the CI fails here?

@codecov
Copy link

codecov bot commented Jan 19, 2023

Codecov Report

Base: 96.99% // Head: 97.12% // Increases project coverage by +0.13% 🎉

Coverage data is based on head (7be8779) compared to base (8d2a3e4).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #82      +/-   ##
==========================================
+ Coverage   96.99%   97.12%   +0.13%     
==========================================
  Files          17       18       +1     
  Lines        1296     1324      +28     
==========================================
+ Hits         1257     1286      +29     
+ Misses         39       38       -1     
Impacted Files Coverage Δ
...lue_astronomy/translators/tests/test_spectrum1d.py 100.00% <ø> (ø)
glue_astronomy/spectral_coordinates.py 100.00% <100.00%> (+6.66%) ⬆️
glue_astronomy/tests/test_spectral_coordinates.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@pllim
Copy link
Contributor

pllim commented Jan 19, 2023

I tried this out at spacetelescope/jdaviz#1968 but now it fails with an even more obscure error:

AttributeError: 'numpy._ArrayFunctionDispatcher' object has no attribute '__code__'

https://github.com/spacetelescope/jdaviz/actions/runs/3959079839/jobs/6781436962

@dhomeier
Copy link
Contributor

spectrum1d test was still expecting the old, incorrect shape.

@pllim
Copy link
Contributor

pllim commented Jan 19, 2023

Oh wait... glue-astronomy==0.5.1 ... I think I have to remove pin because I am trying to install it from fork branch that has no tags. Hold on.

Copy link
Contributor

@dhomeier dhomeier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixes the jdaviz test suite for me (macos-py310), but should probably wait for @pllim's test PR to at least run.

Copy link
Contributor

@pllim pllim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this fixes the devdeps job in Jdaviz. Thanks!

@dhomeier dhomeier changed the title Fixed return type for pixel <-> world conversions in SpectralCoordinates Fixed return type and shape for pixel <-> world conversions in SpectralCoordinates Jan 19, 2023
@dhomeier dhomeier merged commit 1c535cf into glue-viz:main Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants