-
Notifications
You must be signed in to change notification settings - Fork 73
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
Fix cube fitting compatibility with unit conversion #3190
Merged
Merged
Changes from all commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
645c254
Fix NaN handling in cube fitting and initial fixes for unit conversio…
rosteen 79081d1
Remove debugging prints, add comment for context
rosteen 7520892
Codestyle, changelog
rosteen 7d0823b
Reestimate parameters when cube fitting is toggled
rosteen ee5a108
Only reestimate if spectral y type isn't SB
rosteen c94ef87
Codestyle
rosteen c24b6c2
Changelog
rosteen dabcc84
Fix initializing linear component for cube fit
rosteen a7046c4
Handle linear component estimation for cube case
rosteen a42173c
Only reshape here in 3D case
rosteen f405609
Skip tests that need #3156
rosteen d8972b3
Respect selected display units when initializing model components
rosteen fcba42b
Use app._get_display_unit instead of relying on Unit Conversion
rosteen 9abc3bf
Add equivalency here
rosteen 4a1a10f
Only reestimate here if sb unit != spectral_y unit
rosteen 5a0b525
Don't automatically reestimate when toggling cube fit, make the user …
rosteen a50c940
Remove print
rosteen d3dbcd9
Check for warning in test after cube toggle
rosteen 529bb74
Add test for cube fitting after flux unit change
rosteen 35d2d96
Add a to-do about a test for unit conversion with equivalency
rosteen f46e9d0
Fix failing test
rosteen b33dcc0
Back to parallel processing post-debugging
rosteen File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Why does the spectral_y_unit matter for cube fit? I thought it shouldn't matter what the spectral axis is set to for a non-cube fit. Cube fit should only care about what the data units are (flux or SB, but will shortly always be SB), and the chosen flux/angle display unit.
Since this is 'cube fit change' i can see how this makes sense when cube fit is toggled off, but isn't this also triggered when cube fit is toggled on?
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.
I'm using it a few lines down to check to see if the collapsed spectrum is in flux or surface brightness, to see if the parameter units will be compatible when toggling
cube_fit
.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.
ok that makes sense thanks!