jdaviz icon indicating copy to clipboard operation
jdaviz copied to clipboard

provide access to API from show_launcher

Open kecnry opened this issue 2 years ago • 2 comments
trafficstars

Description

This pull request provides access to the launcher/helper APIs when calling show_launcher(). While working on #2417, I noticed it was difficult to debug because show_launcher only calls show but does not return the launcher itself. But once the launcher is returned, we want a way to be able to access the resulting config-helper as well. This implements the following:

  • instantiated launcher object returned by show_launcher() (launcher = show_launcher())
  • launcher.jdaviz_helper is available after choosing config, and will raise error until a config is chosen
  • calling launcher (as the last entry in a cell) will not re-show a broken launcher after config chosen, but rather a warning message suggesting accessing the chosen configuration via launcher.jdaviz_helper

Note: I do not necessarily suggest we publicize this too much on docs since it is still a bit clunky, but definitely is useful for debugging internally. Ultimately, most users using the launcher will not be calling from the notebook but rather from the standalone mode.

Change log entry

  • [x] 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.

~Once #2417 is merged, this should be rebased and 2418 added to the same changelog entry (if we want this)~

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.

  • [x] 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?
  • [x] 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)?

kecnry avatar Sep 01 '23 17:09 kecnry

Codecov Report

Attention: Patch coverage is 37.50000% with 5 lines in your changes missing coverage. Please review.

Project coverage is 90.77%. Comparing base (51e65ef) to head (0cdd332). Report is 778 commits behind head on main.

Files Patch % Lines
jdaviz/core/launcher.py 37.50% 5 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2418      +/-   ##
==========================================
+ Coverage   90.29%   90.77%   +0.47%     
==========================================
  Files         159      160       +1     
  Lines       18454    19365     +911     
==========================================
+ Hits        16663    17578     +915     
+ Misses       1791     1787       -4     

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

codecov[bot] avatar Sep 01 '23 17:09 codecov[bot]

As stated in the description, at least from a developer-perspective, it's convenient to be able to access the launcher internals for debugging. I do not expect users to ever invoke the launcher this way, since its just as easy to actually call the specific config-helper directly and not have to go through the extra layer of the launcher.

kecnry avatar Sep 05 '23 17:09 kecnry