glue-viz / glue-astronomy

Plugin to add astronomy-specific functionality to glue
https://glue-astronomy.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
11 stars 12 forks source link

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

Closed astrofrog closed 1 year ago

astrofrog commented 1 year ago

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

pllim commented 1 year ago

Is it a concern that the CI fails here?

codecov[bot] commented 1 year ago

Codecov Report

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

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 ```diff @@ 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](https://codecov.io/gh/glue-viz/glue-astronomy/pull/82?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glue-viz) | Coverage Δ | | |---|---|---| | [...lue\_astronomy/translators/tests/test\_spectrum1d.py](https://codecov.io/gh/glue-viz/glue-astronomy/pull/82?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glue-viz#diff-Z2x1ZV9hc3Ryb25vbXkvdHJhbnNsYXRvcnMvdGVzdHMvdGVzdF9zcGVjdHJ1bTFkLnB5) | `100.00% <ø> (ø)` | | | [glue\_astronomy/spectral\_coordinates.py](https://codecov.io/gh/glue-viz/glue-astronomy/pull/82?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glue-viz#diff-Z2x1ZV9hc3Ryb25vbXkvc3BlY3RyYWxfY29vcmRpbmF0ZXMucHk=) | `100.00% <100.00%> (+6.66%)` | :arrow_up: | | [glue\_astronomy/tests/test\_spectral\_coordinates.py](https://codecov.io/gh/glue-viz/glue-astronomy/pull/82?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glue-viz#diff-Z2x1ZV9hc3Ryb25vbXkvdGVzdHMvdGVzdF9zcGVjdHJhbF9jb29yZGluYXRlcy5weQ==) | `100.00% <100.00%> (ø)` | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glue-viz). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glue-viz)

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

pllim commented 1 year ago

I tried this out at https://github.com/spacetelescope/jdaviz/pull/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 commented 1 year ago

spectrum1d test was still expecting the old, incorrect shape.

pllim commented 1 year ago

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.