Line Analysis Plugin now listens to unit conversion changes #3107
Conversation
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
github-actions
bot
added
cubeviz
specviz
testing
imviz
plugin
haticekaratay
force-pushed
the
line-analysis-flux
branch
from
11902d3 to
4cf5462
Compare
kecnry
reviewed
Jul 23, 2024
| if flux_unit == u.dimensionless_unscaled: | ||
| add_flux = True | ||
| flux_unit = u.Unit(self.spectrum_viewer.state.y_display_unit) | ||
| else: | ||
| add_flux = False | ||
| # If the flux unit is equivalent to Jy, or Jy per spaxel for Cubeviz, | ||
| # enforce integration in frequency space | ||
| flux_unit = u.Unit(flux_unit) |
There was a problem hiding this comment.
would it be useful if get_display_unit had an option to cast to the unit itself?
There was a problem hiding this comment.
although that is triggering a few test failures (sigh), so we may need to postpone that work, for now maybe it makes sense to just case to unit immediately so if/when we do implement that it will be easy to notice and remove the duplication
flux_unit = u.Unit(self.app._get_display_unit('flux'))
9 tasks
pllim
reviewed
Jul 24, 2024
Comment on lines
274
to
275
| if (np.any(np.isnan(result.uncertainty.value)) or | ||
| np.any(np.isinf(result.uncertainty.value))): |
There was a problem hiding this comment.
Why not use np.all(np.isfinite()) combo?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
This pull request adds the functionality of dynamically listening to the changes of unit conversion in the line analysis plugin. The outputs for flux are responsive to the changes in the flux dropdown.
Screen.Recording.2024-07-24.at.10.04.24.AM.mov
Fixes #
Change log entry
CHANGES.rst? If you want to avoid merge conflicts,list the proposed change log here for review and add to
CHANGES.rstbefore merge. If no, maintainershould add a
no-changelog-entry-neededlabel.Checklist for package maintainer(s)
This checklist is meant to remind the package maintainer(s) who will review this pull request of some common things to look for. This list is not exhaustive.
triviallabel.