footprints: filter by observatory #3345

kecnry · 2024-12-10T16:19:40Z

Description

This pull request extends on #3322 by allowing to filter by mission (JWST or Roman) via buttons in the UI or the preset_obs select component in the API.

Screen.Recording.2024-12-10.at.11.12.44.AM.mov

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.

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. Bugfix milestone also needs an accompanying backport label.
After merge, any internal documentations need updating (e.g., JIRA, Innerspace)?

kecnry · 2024-12-10T16:24:52Z

jdaviz/core/template_mixin.py

+    @observe('filters')
+    def _update_items(self, msg={}):
+        all_items = [self._to_item(opt) for opt in self.manual_options]
+        self.items = [item for item in all_items if self._is_valid_item(item)]


adding this to the base class then requires renaming all the inheriting classes to have their methods renamed so that they properly override this instead of resulting in duplicated functionality.

* manual options can be either strings or dictionaries * filters passed to SelectPluginComponent will act on manual items, classes that subclass SelectPluginComponent may choose to ALWAYS include all manual items, ignoring filters

* and expose to user API

Co-authored-by: Jennifer Kotler <[email protected]>

* manual options always last * actually assign to correct select

kecnry · 2024-12-19T16:25:52Z

I'm stumped by these test failures (seems the remote data builds are failing to find the new icons for some reason), but will investigate. Reviews should be able to continue in the meantime - I expect the fix to be pretty minor as far as the diff is concerned.

pllim · 2024-12-19T16:31:42Z

Are you sure we do not have to include them as package data?

pllim · 2024-12-19T16:34:44Z

Maybe we need to add "data/*/*" here?

jdaviz/pyproject.toml

Line 115 in a0d77f5

"jdaviz.configs.imviz.tests" = [

pllim · 2024-12-19T16:36:42Z

p.s. I am really not sure. Can you reproduce this locally with remote data flag?

kecnry · 2024-12-19T17:11:03Z

may have just been a case issue, although no idea why that didn't complain locally or on the other tests 🤷‍♂️

pllim · 2024-12-19T17:37:50Z

Ah, okay! OSX (which is probably what you using) is case-insensitive but Linux is case-sensitive.

kecnry · 2024-12-19T19:53:28Z

jdaviz/configs/default/plugins/export/export.py

-            subset = self.app.get_subsets(self.subset.selected)
+            try:
+                subset = self.app.get_subsets(self.subset.selected)
+            except Exception:
+                # subset invalid message will already be set,
+                # no need to set valid/invalid formats.
+                return


this is needed here because adding support for filtering the select dropdowns meant that this line was getting triggered and raising an exception. Without this PR, this traceback could still be triggered by going to the export plugin on an invalid subset (e.g., AND logic but not overlapping), but now it was being triggered in the test when importing the subset instead of when accessing it.

kecnry · 2024-12-20T15:11:11Z

jdaviz/data/icons/jwst.svg

technically these aren't used, but I figure they might be useful at some point and don't really take much space. I can remove from here though if we'd rather wait until we need them?

rosteen

LGTM, I like the icons. Might have to rethink those if we eventually support more predefined mission footprints but it's fine for now.

bmorris3

This works great. I'm a little bummed how often we have to customize and call _update_items (formerly known as _update_layer_items), but hopefully it'll asymptotically converge.

Thanks again to @Jenneh too for the icons!

codecov · 2024-12-20T18:37:03Z

Codecov Report

Attention: Patch coverage is 85.21739% with 17 lines in your changes missing coverage. Please review.

Project coverage is 88.70%. Comparing base (297f8db) to head (53dcc7d).
Report is 8 commits behind head on main.

Files with missing lines	Patch %	Lines
...viz/configs/imviz/plugins/footprints/footprints.py	58.33%	10 Missing ⚠️
jdaviz/configs/default/plugins/export/export.py	68.75%	5 Missing ⚠️
jdaviz/core/template_mixin.py	97.01%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3345      +/-   ##
==========================================
- Coverage   88.76%   88.70%   -0.06%     
==========================================
  Files         125      125              
  Lines       19231    19291      +60     
==========================================
+ Hits        17071    17113      +42     
- Misses       2160     2178      +18

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

kecnry added this to the 4.1 milestone Dec 10, 2024

github-actions bot added imviz plugin Label for plugins common to multiple configurations labels Dec 10, 2024

kecnry commented Dec 10, 2024

View reviewed changes

kecnry force-pushed the wfi-footprint-filters branch from 82203ff to ceab93f Compare December 13, 2024 19:08

kecnry and others added 11 commits December 18, 2024 11:56

allow manual_options to be filter

910d177

* manual options can be either strings or dictionaries * filters passed to SelectPluginComponent will act on manual items, classes that subclass SelectPluginComponent may choose to ALWAYS include all manual items, ignoring filters

FileImportSelect icon support

c128a4d

use JWST and Roman footprints

aa03ca2

toggle buttons for telescope filters

f5eb10f

WIP: replace icons with text

8d9fabf

use select component instead of boolean toggles

cc4042f

* and expose to user API

update on rebase

9a56a51

revert to use icon in dropdown

b91b7a4

use custom icons

d1077ca

Co-authored-by: Jennifer Kotler <[email protected]>

codestyle

64d7e4c

add to existing changelog entry

056271c

kecnry force-pushed the wfi-footprint-filters branch 7 times, most recently from c1b3878 to f135ad5 Compare December 18, 2024 18:42

kecnry added 2 commits December 18, 2024 14:01

fix test failures

504f25c

handle exception in export plugin

bfdeb50

kecnry force-pushed the wfi-footprint-filters branch from f135ad5 to bfdeb50 Compare December 18, 2024 19:31

kecnry added 2 commits December 18, 2024 14:41

codestyle

5c41849

correct ordering of options

f915bf5

* manual options always last * actually assign to correct select

kecnry marked this pull request as ready for review December 18, 2024 20:15

kecnry requested a review from rosteen as a code owner December 18, 2024 20:15

kecnry requested review from javerbukh, pllim, bmorris3, haticekaratay, cshanahan1 and gibsongreen as code owners December 18, 2024 20:15

fix case of referenced image filename

53dcc7d

kecnry force-pushed the wfi-footprint-filters branch from 6a4ac6d to 53dcc7d Compare December 19, 2024 17:10

kecnry commented Dec 19, 2024

View reviewed changes

kecnry commented Dec 20, 2024

View reviewed changes

rosteen approved these changes Dec 20, 2024

View reviewed changes

kecnry added the Ready for final review label Dec 20, 2024

bmorris3 approved these changes Dec 20, 2024

View reviewed changes

kecnry merged commit 9663cf7 into spacetelescope:main Dec 20, 2024
23 checks passed

kecnry deleted the wfi-footprint-filters branch December 20, 2024 17:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

footprints: filter by observatory #3345

footprints: filter by observatory #3345

kecnry commented Dec 10, 2024 •

edited

Loading

kecnry Dec 10, 2024

kecnry commented Dec 19, 2024

pllim commented Dec 19, 2024

pllim commented Dec 19, 2024

pllim commented Dec 19, 2024

kecnry commented Dec 19, 2024

pllim commented Dec 19, 2024

kecnry Dec 19, 2024

kecnry Dec 20, 2024

rosteen left a comment

bmorris3 left a comment

codecov bot commented Dec 20, 2024

footprints: filter by observatory #3345

footprints: filter by observatory #3345

Conversation

kecnry commented Dec 10, 2024 • edited Loading

Description

Change log entry

Checklist for package maintainer(s)

kecnry Dec 10, 2024

Choose a reason for hiding this comment

kecnry commented Dec 19, 2024

pllim commented Dec 19, 2024

pllim commented Dec 19, 2024

pllim commented Dec 19, 2024

kecnry commented Dec 19, 2024

pllim commented Dec 19, 2024

kecnry Dec 19, 2024

Choose a reason for hiding this comment

kecnry Dec 20, 2024

Choose a reason for hiding this comment

rosteen left a comment

Choose a reason for hiding this comment

bmorris3 left a comment

Choose a reason for hiding this comment

codecov bot commented Dec 20, 2024

Codecov Report

kecnry commented Dec 10, 2024 •

edited

Loading