spacetelescope / jdaviz

JWST astronomical data analysis tools in the Jupyter platform
https://jdaviz.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
138 stars 73 forks source link

fix formatting of unit after appending angle #3198

Closed cshanahan1 closed 1 week ago

cshanahan1 commented 2 weeks ago

This PR fixes a traceback when changing flux units via API. When changing between Jy and erg/c/cm2/A, the traceback said the unit was not in the list of available options for the y axis because the strings differed. This happens when handing contours. To reproduce:

from jdaviz.conftest import _create_spectrum1d_cube_with_fluxunit from jdaviz import Cubeviz import astropy.units as u cube = _create_spectrum1d_cube_with_fluxunit(shape=(25, 25, 4), fluxunit=u.Jy / u.sr, with_uncerts=True) cubeviz_helper = Cubeviz() cubeviz_helper.load_data(cube, data_label="test") uc = cubeviz_helper.plugins['Unit Conversion']._obj uc.flux_unit.selected='erg / (Angstrom s cm2)'

In the unit conversion plugin, the '_append_angle_correctly' method to combine chosen flux and angle units sometimes produces unit strings that aren't in the required order for setting the spectral axis. These options are hard coded in UnitConverterWithSpectral (app.py). Additionally, this method was setting the sb_unit_selected read-only attribute twice in a row, so this redundancy was removed.

There is a remaining traceback coming from contours when changing flux unit, which is described in JDAT-4785.

codecov[bot] commented 2 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 89.60%. Comparing base (d49e3b5) to head (e824d32). Report is 3 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #3198 +/- ## ========================================== + Coverage 88.50% 89.60% +1.09% ========================================== Files 124 124 Lines 18448 20869 +2421 ========================================== + Hits 16327 18699 +2372 - Misses 2121 2170 +49 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

pllim commented 2 weeks ago

Maybe add your reproducing code to regression test suite?

cshanahan1 commented 2 weeks ago

closing this, it's fixed by 3192