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

Correct the linking for 1D model results in Cubeviz #1531

Merged
merged 1 commit into from
Jul 29, 2022

Conversation

rosteen
Copy link
Collaborator

@rosteen rosteen commented Jul 29, 2022

This should have been grabbing the last world coordinate, not the first, in case the reference data is multi-dimensional.

Closes #1530

@rosteen rosteen added no-changelog-entry-needed changelog bot directive bug Something isn't working cubeviz labels Jul 29, 2022
@rosteen
Copy link
Collaborator Author

rosteen commented Jul 29, 2022

I'm marking this no-changelog-needed since the bug was never released.

Copy link
Member

@kecnry kecnry left a comment

Choose a reason for hiding this comment

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

Fixes #1530, thanks!

@kecnry kecnry mentioned this pull request Jul 29, 2022
9 tasks
@codecov
Copy link

codecov bot commented Jul 29, 2022

Codecov Report

Merging #1531 (4d5142d) into main (e409730) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1531   +/-   ##
=======================================
  Coverage   85.34%   85.34%           
=======================================
  Files          91       91           
  Lines        8660     8660           
=======================================
  Hits         7391     7391           
  Misses       1269     1269           
Impacted Files Coverage Δ
jdaviz/app.py 92.39% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e409730...4d5142d. Read the comment docs.

@kecnry
Copy link
Member

kecnry commented Jul 29, 2022

regarding changelog - should we put this PR number in the original changelog entry where it was introduced since it basically modifies that PR?

@rosteen
Copy link
Collaborator Author

rosteen commented Jul 29, 2022

Good idea, I'll do that.

@rosteen rosteen force-pushed the fix-cubeviz-model-fitting branch from 2843d2c to dd18ce4 Compare July 29, 2022 15:53
@rosteen rosteen force-pushed the fix-cubeviz-model-fitting branch from dd18ce4 to 4d5142d Compare July 29, 2022 15:54
@rosteen rosteen removed the no-changelog-entry-needed changelog bot directive label Jul 29, 2022
@rosteen rosteen added this to the 2.9 milestone Jul 29, 2022
Copy link
Contributor

@javerbukh javerbukh 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 bug, thanks!

@javerbukh javerbukh merged commit 6e63d1d into spacetelescope:main Jul 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cubeviz
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Cubeviz: Model fitting broken with spectral+spatial subsets
3 participants