re-usable component to show quantity with significant decimal digits

[kecnry]

Description

This pull request creates yet another re-usable component (used in the output from line analysis for now), to have the UI round quantities based on the uncertainty or a default precision. If an uncertainty is available, both the value and uncertainty are rounded to the second significant decimal digit in the uncertainties (currently no rounding is done for digits left of the decimal). Note that this might not be entirely perfect - javascript treatment of numbers can be a bit funny... but the full underlying floats are untouched via the API.

Before and after screenshots in line analysis:

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.

• Are two approvals required? Branch protection rule does not check for the second approval. If a second approval is not necessary, please apply the trivial label.
• Do the proposed changes actually accomplish desired goals? Also manually run the affected example notebooks, if necessary.
• Do the proposed changes follow the STScI Style Guides?
• Are tests added/updated as required? If so, do they follow the STScI Style Guides?
• Are docs added/updated as required? If so, do they follow the STScI Style Guides?
• Did the CI pass? If not, are the failures related?
• Is a change log needed? If yes, is it added to CHANGES.rst?
• Is a milestone set?
• After merge, any internal documentations need updating (e.g., JIRA, Innerspace)?

[codecov]

Codecov Report

Merging #1244 (d32361a) into main (9e50df2) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1244   +/-   ##
=======================================
  Coverage   79.62%   79.62%           
=======================================
  Files          90       90           
  Lines        7209     7224   +15     
=======================================
+ Hits         5740     5752   +12     
- Misses       1469     1472    +3     

Impacted Files	Coverage Δ
...igs/specviz/plugins/line_analysis/line_analysis.py	78.72% <ø> (ø)
jdaviz/app.py	91.16% <100.00%> (+0.01%)	⬆️
...imviz/plugins/aper_phot_simple/aper_phot_simple.py	88.78% <0.00%> (-0.76%)	⬇️
...igs/default/plugins/model_fitting/model_fitting.py	34.65% <0.00%> (ø)
jdaviz/utils.py	89.65% <0.00%> (+0.29%)	⬆️

---

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 9e50df2...d32361a. Read the comment docs.

[javerbukh]

I'm trying this PR with the example notebooks in a new conda environment and I'm getting NaNs in line analysis. I'm creating a subset in the spectrum viewer (in both cubeviz and specviz) and setting that as the spectral region in line analysis.

[kecnry]

I'm getting NaNs in line analysis

This should be fixed now. My local testing branch was forcing vue to re-compute the data entries, but I now re-factored to force it to "watch" input changes and re-compute the rounded displayed values.

[javerbukh]

Getting a mix of 0s and NaNs now.

[javerbukh]

Works well in Specviz though! Is it not working the same in cubeviz because the spectrum does not have an uncertainty value?

[kecnry]

Ah, this is a case where the original specs aren't sufficient. We probably don't want to default to 5 (or 8) decimal places after all, but rather significant digits. These are showing as 0s because the underyling float is smaller than that. I'll try to rework the logic to work on significant digits (or decimals in scientific notation) instead.

[kecnry]

Ok, I rewrote the logic entirely to be based on precision instead of number of decimal places and haven't managed to find a case where it breaks, but please do try! If this isn't robust enough... we may have to handle it on the python side, Javascript can be a pain to deal with numbers.

[javerbukh]

Before I create a subset, everything looks as expected in cubeviz. But after selecting a subset, I get the following screen. Everything works well in specviz.

.

[kecnry]

Can you compare to the list of dictionaries storing the results (viz.app.get_tray_item_from_name('specviz-line-analysis').results)? If those show empty entries, nans, etc, then the formatting isn't the issue and I think its probably out-of-scope here. If the dictionary does show results and the formatting is somehow choking and showing NaNs, then I should be able to track down what's going on if you send me the values.

[javerbukh]

Tried it out and you are correct, the results on the backend are NaNs and 0s. I'll go ahead and approve since everything else is working as expected!

[{'function': 'Line Flux',
  'result': '9.607043970686176e-07',
  'uncertainty': '',
  'unit': '1e-17 erg m / (Angstrom cm2 s)'},
 {'function': 'Equivalent Width',
  'result': '-1.6298725716548473e-09',
  'uncertainty': '',
  'unit': 'm'},
 {'function': 'Gaussian Sigma Width',
  'result': 'nan',
  'uncertainty': '',
  'unit': 'm'},
 {'function': 'Gaussian FWHM',
  'result': 'nan',
  'uncertainty': '',
  'unit': 'm'},
 {'function': 'Centroid',
  'result': '6.675966140764623e-07',
  'uncertainty': '',
  'unit': 'm'}]

[duytnguyendtn]

I ran into an Jesse's nan issue, but when I extracted the parameters, the line flux actually returns a value, which isn't showed in the UI (and as usual, Mosviz is the problem config again 😅). Thoughts @kecnry?

[kecnry]

@duytnguyendtn - interesting case. The logic is choking because of the 0 uncertainty. How do we want to handle this case (perhaps display the default 8 digits... but do we show or hide the +/- 0... or is this an upstream issue in specutils that should not return a zero uncertainty)?

[duytnguyendtn]

I interpret 0 uncertainty as the same as not having an uncertainty. I would just drop the 0+/-, similar to how the centroid appears in the above screenshot. Maybe a PO is needed here?

[kecnry]

I like that solution for now (and implemented it in the latest commit), and will open a follow-up ticket for investigating why the flux is returning 0 uncertainty in the first place. 🐱

[duytnguyendtn]

Thanks for the quick fix! Looks good to me