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

Add more flux units for SB conv #3389

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ Cubeviz

- Fixed copious warnings from spaxel tool when data has INF. [#3368]

- Fixed unit conversion for certain units when Surface Brightness is selected. [#3389]

Imviz
^^^^^

Expand Down
29 changes: 19 additions & 10 deletions jdaviz/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -326,9 +326,19 @@ def standardize_roman_metadata(data_model):


def indirect_units():
from jdaviz.core.custom_units_and_equivs import PIX2
from jdaviz.core.unit_conversion_utils import supported_sq_angle_units

units = []
# Only PIX2 here because sr would mess with PIXAR_SR stuff from JWST.
units = [
u.Jy / PIX2,
u.MJy / PIX2,
u.mJy / PIX2,
u.uJy / PIX2,
u.W / (u.Hz * u.m**2 * PIX2),
u.eV / (u.Hz * u.s * u.m**2 * PIX2),
u.ct / PIX2
]

for angle_unit in supported_sq_angle_units():
units += [
Expand Down Expand Up @@ -418,11 +428,10 @@ def flux_conversion(values, original_units, target_units, spec=None, eqv=None, s
solid_angle_in_targ = check_if_unit_is_per_solid_angle(targ_units, return_unit=True)

# Ensure a spectrum passed through Spectral Extraction plugin
if ((((spec and ('_pixel_scale_factor' in spec.meta))) or
(spectral_axis is not None and ('_pixel_scale_factor' in spectral_axis.info.meta)))
and
(((solid_angle_in_orig) and (not solid_angle_in_targ)) or
((not solid_angle_in_orig) and (solid_angle_in_targ)))):
if (((spec and ('_pixel_scale_factor' in spec.meta)) or
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Too many parenthesis so I removed some. No functional change.

Copy link
Contributor

Choose a reason for hiding this comment

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

From what was discussed at tag up, I'll mention @cshanahan1 has a ticket that consolidates the flux_conversion here, and flux_conversion_general function so there will be some refactoring down the line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, should I revert?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think so, just wanted to mention it here. The plan is to remove one of the two functions (I'm not sure which), none of the changes are major conflicts here and it may just get removed entirely so I'm good with keeping the changes here for now.

(spectral_axis is not None and ('_pixel_scale_factor' in spectral_axis.info.meta))) and
((solid_angle_in_orig and (not solid_angle_in_targ)) or
((not solid_angle_in_orig) and solid_angle_in_targ))):
# Data item in data collection does not update from conversion/translation.
# App-wide original data units are used for conversion, original and
# target_units dictate the conversion to take place.
Expand Down Expand Up @@ -456,9 +465,9 @@ def flux_conversion(values, original_units, target_units, spec=None, eqv=None, s
# additional conversions to reach the desired end unit.
# if spec_unit in [original_units, target_units]:
result = _indirect_conversion(
values=values, orig_units=orig_units, targ_units=targ_units,
eqv=eqv, spec_unit=spec_unit
)
values=values, orig_units=orig_units, targ_units=targ_units,
eqv=eqv, spec_unit=spec_unit
)

if result and len(result) == 2:
values, updated_units = result
Expand Down Expand Up @@ -523,7 +532,7 @@ def _indirect_conversion(values, orig_units, targ_units, eqv,
targ_units /= solid_angle_in_spec
solid_angle_in_targ = solid_angle_in_spec
if ((u.Unit(targ_units) in indirect_units()) or
(u.Unit(orig_units) in indirect_units())):
(u.Unit(orig_units) in indirect_units())):
# SB -> Flux -> Flux -> SB
temp_orig = orig_units * solid_angle_in_orig
temp_targ = targ_units * solid_angle_in_targ
Expand Down
Loading