Add docs page to explain how to create jdaviz-readable products #2228

camipacifici · 2023-05-31T18:36:29Z

Description

This pull request is to add a documentation page to explain how to create products that can be read into Jdaviz. This is to address a request from the community.

Change log entry

Is a change log needed? If yes, is it added to CHANGES.rst? If you want to avoid merge conflicts,
list the proposed change log here for review and add to CHANGES.rst before merge. If no, maintainer
should add a no-changelog-entry-needed label.

Under "Other changes": documentation page to explain how to create products that can be read into Jdaviz.

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 milestone set? Set this to bugfix milestone if this is a bug fix and needs to be released ASAP; otherwise, set this to the next major release milestone.
After merge, any internal documentations need updating (e.g., JIRA, Innerspace)?

pllim

Do we really need a new page or should this be a new section in https://github.com/spacetelescope/jdaviz/blob/main/docs/specviz/import_data.rst ? Has this been discussed?

docs/create_products.rst

codecov · 2023-05-31T19:01:19Z

Codecov Report

Patch coverage has no change and project coverage change: -0.01 ⚠️

Comparison is base (dad1bd9) 91.59% compared to head (875fa52) 91.59%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2228      +/-   ##
==========================================
- Coverage   91.59%   91.59%   -0.01%     
==========================================
  Files         148      148              
  Lines       16496    16515      +19     
==========================================
+ Hits        15110    15127      +17     
- Misses       1386     1388       +2

Impacted Files	Coverage Δ
jdaviz/configs/cubeviz/helper.py	`98.87% <ø> (ø)`

... and 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

camipacifici · 2023-05-31T21:10:09Z

This is supposed to apply to all data, not just 1D spectra. I do not see it fitting well under Specviz.
Maybe I should make that clearer?

pllim · 2023-05-31T22:16:56Z

This is supposed to apply to all data

Certainly not images? At least I don't see any mention of images or image cubes.

camipacifici · 2023-06-01T01:17:52Z

You are right, it does not obviously apply to images. I specified at the beginning that it applies to 1D, 2D, and 3D spectroscopic data though.

kecnry · 2023-06-01T13:26:26Z

Do we really need a new page or should this be a new section in https://github.com/spacetelescope/jdaviz/blob/main/docs/specviz/import_data.rst ?

I think this does make sense as a separate page since (if) it is targeted at developers/contributors more-so than users. I'd be worried about these many "advanced tangents" living on a main page about loading data, although we could link from there to here.

pllim

As discussed, please also link to this page from docs/.../import_data.rst for Cubeviz, Mosviz, Specviz, and Specviz2D so it is more easily discoverable. You can link using this directive below:

:ref:`create_products`

If you choose to also accept my suggestions via GitHub UI, please do it in batch mode that is available under "Files changed". Alternately, you can also choose to manually apply the suggested changes as you see fit by editing the files directly on your local checkout.

Thank you!

docs/create_products.rst

Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com>

camipacifici · 2023-06-03T01:32:41Z

I accepted all the suggestions (thank you!) and added the reference in the various import_data pages.
Thank you again for the review!

pllim

LGTM. Thanks!

docs/create_products.rst

rosteen

LGTM other than one minor suggestion. I'm assuming there won't be any objections to it, so I may just commit it and merge the PR.

docs/create_products.rst

* Added .ico and .svg files * app-wide display unit in specviz (#2048) * basic app-wide display unit in specviz (not all plugins are updated yet) * unit-aware select component which maps to glue-supported unit strings * implement use_display_units option for get_data, get_subsets Co-authored-by: Jesse Averbukh <javerbukh@gmail.com> Co-authored-by: Kyle Conroy <kyleconroy@gmail.com> * bump glue-core to necessary version for display units * change PR number in changelog to main PR * Display units line analysis (#2128) * emit event when display unit changed * update line analysis for unit changes * cubeviz/slice plugin unit conversion support (we might want to generalize some of this logic more into the mark's _update_data when adding support for spectral lines as well) * Display units support for markers plugin (#2130) * update markers plugin for display units * move unit-updating logic to mark itself * LineUncertainties to be unit-aware * fix support for markers in cubeviz * display units: fix failing tests (#2132) * fix error raised when spectrum uncertainty is None * fix RTD build failure because of missing mixin available in import * fix test setting display unit now that um is preferred to micron * add disabled unit conversion plugin to mosviz * avoid unit-conversion error in cubeviz tests * bump versions of glue/glue-jupyter * fix deg intialization in cubeviz slice plugin * add LinesAutoUnit to __all__ so RTD can find it * add use_display_units kwarg to get_spectral_regions * fix setting display units for unitless data * Apply suggestions from code review Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com> * Display units: line lists (#2148) * BaseSpectrumVerticalLine to inherit unit-support from PluginMark * convert units on internal rest value * code cleanup * remove unit conversion plugin from all bug specviz * for initial implementation - eventually we'll want to implement across all plugins/viewers * get_display_unit fallback for configs without unit conversion plugin * Update glue-core dependency * Update Subset Tools plugin to use display units (#2195) * bump glue-core to necessary version for display units * cubeviz/slice plugin unit conversion support (we might want to generalize some of this logic more into the mark's _update_data when adding support for spectral lines as well) * display units: fix failing tests (#2132) * fix error raised when spectrum uncertainty is None * fix RTD build failure because of missing mixin available in import * fix test setting display unit now that um is preferred to micron * add disabled unit conversion plugin to mosviz * avoid unit-conversion error in cubeviz tests * bump versions of glue/glue-jupyter * fix deg intialization in cubeviz slice plugin * add LinesAutoUnit to __all__ so RTD can find it * add use_display_units kwarg to get_spectral_regions * fix setting display units for unitless data * Apply suggestions from code review Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com> * remove unit conversion plugin from all bug specviz * for initial implementation - eventually we'll want to implement across all plugins/viewers * Working on unit conversion handling in subset plugin * Get subset updating working in non-default units * Add changelog * Update jdaviz/configs/default/plugins/subset_plugin/subset_plugin.vue Co-authored-by: Kyle Conroy <kyleconroy@gmail.com> --------- Co-authored-by: Kyle Conroy <kyleconroy@gmail.com> Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com> * allow downstream apps to set what classes are considered native marks * Fix unit conversion for PluginMarks and y-limits (#2215) * Fix unit conversion for PluginMarks and y-limits * Only use equivalency if it's a spec viewer * Store wavelength unit on data update * Remove redundant line * Better check in marks * Replace subset_to_apply in docs where possible (#2223) * Replace subset_to_apply in docs where possible (#2223) * Avoid trying to convert units of empty marks arrays * Add open method to automatically detect config and load data * Update demo notebooks * Codestyle * Add autoconfig test coverage * Revert "Update demo notebooks" This reverts commit cc513b3. * Add mention of open to relevant example notebooks * Prepare to support open via cli * Remove the need to open file twice for autoconfig detection * Fix Helper vs App Docstring * TST: Change PIP_EXTRA_INDEX_URL because packages moved where they upload nightly builds. * Display units: model fitting (#2216) * model component compatibility based on display units * equation validity based on display units * allow re-estimating model parameters to update compat checks * test coverage * get_data(use_display_units) to pass spectral_density equivalency * handle whitespace in model equation * Add clarity for kwargs * plugin results: support overwriting when data loaded in multiple viewers (#2222) * reverts removal of call to set_plot_axes when loading data into viewer (#2235) * fixes a bug in the axes labels on cube viewers in cubeviz as well as setting the correct attributes in lcviz * Subsume failing identifier test into autoconfig test * Update subset plugin UI (#2234) * Update subset plugin UI * Apply suggestions from code review Co-authored-by: Kyle Conroy <kyleconroy@gmail.com> --------- Co-authored-by: Kyle Conroy <kyleconroy@gmail.com> * Changelog * Add docs page to explain how to create jdaviz-readable products (#2228) * Add page for data products * Update index * Fix title * Capital letters * Address first review comments * Edit format * Apply suggestions from code review Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com> * Including reference in import_data * Update docs/create_products.rst * Update docs/create_products.rst --------- Co-authored-by: Camilla Pacifici <cpacifici@stsci.edu> Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com> Co-authored-by: Ricky O'Steen <39831871+rosteen@users.noreply.github.com> * FEAT: Load annulus from file and allow IMPORT DATA to load reg files. Annulus support provided by glue-viz/glue#2403 and glue-viz/glue-astronomy#92 Add test for exception. Bump minimum requirements for glue-core and glue-astronomy. * disallow gaussian smooth output as input * BUG: Fix ellipse translation in app.get_subsets() * DOC: Use pydata-sphinx-theme (#2243) * DOC: Use pydata-sphinx-theme that has dark mode, modern design, and mobile friendly DOC: No longer need tomli DOC: Now requires sphinx-astropy 1.9.1 * Add cards to index page with icons * Remove iframe hardcoding so it scales in mobile mode * DOC: Insert small logo next to title instead of a giant logo above the title * Add app method to simplify spectral subset (#2237) * Add app method to simplify spectral subset Add simplify button to subset plugin * Fix xor bug exposed by simplify button * Fix style checks * Remove print statements * Add tooltip to simplify button * Make button appear only if the subset can be simplified * Make simplify button appear only when applicable * Apply suggestions from code review Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com> * Update changes file --------- Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com> * Fix missed conflict --------- Co-authored-by: Jennifer Kotler <jkotler@esme.stsci.edu> Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com> Co-authored-by: Jesse Averbukh <javerbukh@gmail.com> Co-authored-by: Ricky O'Steen <rosteen@stsci.edu> Co-authored-by: Ricky O'Steen <39831871+rosteen@users.noreply.github.com> Co-authored-by: Duy Nguyen <duytnguyendtn.open@gmail.com> Co-authored-by: Camilla Pacifici <camilla.pacifici@gmail.com> Co-authored-by: Camilla Pacifici <cpacifici@stsci.edu>

Camilla Pacifici added 4 commits May 31, 2023 13:44

Add page for data products

341a553

Update index

5ca97d3

Fix title

4ff288e

Capital letters

ffe960a

camipacifici requested review from duytnguyendtn, rosteen, javerbukh, pllim, kecnry, bmorris3, haticekaratay and cshanahan1 as code owners May 31, 2023 18:36

github-actions bot added the documentation Explanation of code and concepts label May 31, 2023

pllim added this to the 3.6 milestone May 31, 2023

pllim added the no-changelog-entry-needed changelog bot directive label May 31, 2023

pllim reviewed May 31, 2023

View reviewed changes

docs/create_products.rst Outdated Show resolved Hide resolved

docs/create_products.rst Outdated Show resolved Hide resolved

Camilla Pacifici added 2 commits May 31, 2023 21:14

Address first review comments

6a64592

Edit format

c53ad09

pllim reviewed Jun 1, 2023

View reviewed changes

camipacifici and others added 2 commits June 2, 2023 21:13

Apply suggestions from code review

6f48877

Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com>

Including reference in import_data

1a513b0

pllim approved these changes Jun 3, 2023

View reviewed changes

pllim added the Ready for final review label Jun 3, 2023

rosteen reviewed Jun 5, 2023

View reviewed changes

docs/create_products.rst Outdated Show resolved Hide resolved

rosteen approved these changes Jun 5, 2023

View reviewed changes

Update docs/create_products.rst

fe60d15

rosteen reviewed Jun 5, 2023

View reviewed changes

docs/create_products.rst Outdated Show resolved Hide resolved

Update docs/create_products.rst

875fa52

rosteen merged commit 2a35e27 into spacetelescope:main Jun 5, 2023

camipacifici deleted the docs-loaders branch March 27, 2024 19:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add docs page to explain how to create jdaviz-readable products #2228

Add docs page to explain how to create jdaviz-readable products #2228

camipacifici commented May 31, 2023

pllim left a comment

codecov bot commented May 31, 2023 •

edited

Loading

camipacifici commented May 31, 2023

pllim commented May 31, 2023

camipacifici commented Jun 1, 2023

kecnry commented Jun 1, 2023

pllim left a comment

camipacifici commented Jun 3, 2023

pllim left a comment

rosteen left a comment

Add docs page to explain how to create jdaviz-readable products #2228

Add docs page to explain how to create jdaviz-readable products #2228

Conversation

camipacifici commented May 31, 2023

Description

Change log entry

Checklist for package maintainer(s)

pllim left a comment

Choose a reason for hiding this comment

codecov bot commented May 31, 2023 • edited Loading

Codecov Report

camipacifici commented May 31, 2023

pllim commented May 31, 2023

camipacifici commented Jun 1, 2023

kecnry commented Jun 1, 2023

pllim left a comment

Choose a reason for hiding this comment

camipacifici commented Jun 3, 2023

pllim left a comment

Choose a reason for hiding this comment

rosteen left a comment

Choose a reason for hiding this comment

codecov bot commented May 31, 2023 •

edited

Loading