Virtual Observatory plugin for Jdaviz #2872

duytnguyendtn · 2024-05-10T21:17:52Z

Description

NOTE: This PR depends on https://github.com/astropy/pyvo/tree/main until it is released with the changes introduced in astropy/pyvo#594

This PR introduces a new plugin to query resources and load data products published by collaborators of the International Virtual Observatory Alliance. This Plugin currently queries resources implementing the original 1.0 version of the Simple Image Access protocol by specifying a source and a waveband and loads the selected products into Imviz. Though currently only implemented into Imviz, I placed this plugin into the default config in hopes this plugin can be expanded to also work for Specviz via the Simple Spectrum Access Protocol, and Cubeviz as well. Documentation and Unit Tests have been implemented, and this is formally ready to review!

Additionally, I placed the VO plugin closer to the top mainly for my own testing, but also because loading data generally occurs before data analysis. Feel free to adjust the location of the plugin within the plugin bar to your liking.

Thanks for your inputs/help getting us this far! We at the HEASARC hope this is a useful contribution to the Jdaviz tool and 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.

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)? 🐱

…lumns

…lect source before resource, add manual coordinate toggle to viewer dropdown

…istry calls while typing the source name

camipacifici · 2024-07-31T14:01:47Z

Sorry, I just reread your previous message. I do not want to block the user from loading data. I just want to warn them that they will not see the images aligned if they do not link by WCS.

…bled

duytnguyendtn · 2024-07-31T21:36:11Z

@camipacifici I just pushed up 10d712d which provides a warning when:

WCS linking is not enabled and
There is pre-existing data in the data collection

On point 2, I figured a warning about misaligned data would be nonsensical if there's only one dataset total (the one the user is loading). Give it a try and see if it's the behavior you're expecting. Thanks!

camipacifici · 2024-08-01T20:41:24Z

I pulled your latest version here but I do not see the warning. Maybe because I had the plugin already open when I loaded data?

Here is a case that returns a problem:

Any clues? It is also still thinking for some reason, but the rest of the tool is responsive.

TEST: Combine load_data error test to main load_data test

duytnguyendtn · 2024-08-02T15:17:24Z

Hi Cami,

On the smaller points, I pushed up a change that should fix the "hanging" issue. I forcibly stopped the plugin whenever a load data error occurred, but forgot to properly clear the loading flag. Instead, now a load_data error should not stop the plugin; it will notify via snackbar message and then move on to other selected files.

On the lack of linking error message, that one concerns me; let me explicitly spell out what conditions it should hit and you let me know which path you were in, or if you hit a case I'm not covering:

If there is NO data loaded, then the VO plugin loads the first data entry into the data collection, no error message should appear, regardless of link type
If there is already data in the data collection then the VO plugin loads another data, and pixel linking is on, the warning snackbar should appear
If there is already data in the data collection then the VO plugin loads another data, and pixel linking is off, no warning should appear.

I have a test that checks for these conditions in CI, which is passing, so I'm concerned you may have found a scenario I'm not covering for. If you are in one of the above, could you detail what was in your data_collection when you tried to download from the VO, and were expecting the warning?

Lastly, SkyView is down at the moment due to a storage system error so I'll test your other error message once it comes back online. Thanks!

duytnguyendtn · 2024-08-07T15:34:01Z

Hi Cami!

Now that SkyView is back up I was able to start debugging your aforementioned issue. In short, I did get an error, but it was a download timeout error, not the join error your screenshot has. After I pushed 5a61cc9 which effectively disables the timeout, I was able to successfully download the file you specified in the screenshot. Could you try again? I'm wondering if either you ran into an issue with SkyView when we were figuring out our storage failure, or if you found a platform-specific error. I included a code snippet for reproducibility. Thanks!

vo_plugin = imviz.plugins['Virtual Observatory']._obj
vo_plugin.viewer_selected = "Manual"
vo_plugin.source = "186.70493, 21.83403"
vo_plugin.waveband_selected = "optical"
vo_plugin.vue_query_registry_resources()
vo_plugin.resource_selected = "SDSS"
vo_plugin.vue_query_resource()
vo_plugin.table.selected_rows = [vo_plugin.table.items[0]]
vo_plugin.vue_load_selected_data()

jdaviz/configs/default/plugins/virtual_observatory/vo_plugin.py

duytnguyendtn · 2024-09-12T20:21:29Z

Hi @camipacifici! I've implemented basic versions of the two requests you had:

03a00e3 implements the "recalculate center" button for the viewer coordinate select (which only shows when the user is NOT on manual mode)
783262a adds a dropdown to change the units of the search radius (Currently have degrees, radians, arcmins, and arcsecs).

NOTE: I have a pyvo PR ~~open~~ merged but not released to simplify the code needed in Jdaviz to enable (2), so this currently depends on the main branch. Please install main when you try it out.

There are still nice-to-haves I'd like to add when I have a chance, such as detecting when the coordinates pulled are out of date (probably by changing the color of the centering icon), but hopefully this increment gives you enough to try out the behavior. Let me know your thoughts!

duytnguyendtn · 2024-09-13T21:03:09Z

Hi @camipacifici!

Another quick update. So... I know we were concerned about the performance of auto-updating coordinates during pan-zoom, but I was curious and went ahead and implemented it anyways to see how it would perform. Surprisingly, it does quite well! Any lag I feel doesn't seem to get worse if I enable the "following" switch (alternatively, I don't see a lag improvement when I disable the toggle):

Please try it out and see how it performs on your side! I feel like it massively improves the user experience in searching for coordinates, but if it doesn't work for you, I can always remove it.

Thanks!
Duy

… not

camipacifici · 2024-09-16T18:38:09Z

Thank you for all the updates! Apparently, my installation was not updating and that's why I could not see the latest improvements. I now see the warning properly, the new choice for the units, and the recentering (which is surprisingly efficient!).

I went one step further and tried to load two images (using the same search of optical-CANDELS). It loaded them fine and I could link them by WCS, but I saw some weirdness when I tried to change the plot options. I now have 3 layers (A, B, C). Layer B is all black and I have not checked if the image is missing or all black in the database or something else is wrong. When I try to edit layer C, the tool keeps trying to edit layer B. I have not checked yet if this problem is also in main. Here are three screenshots:
The name of the image in layer B.

The name of the image in layer C.

The fact that C is not responding to plot options (eye says it is showing, tab says it is not).

duytnguyendtn · 2024-09-16T18:43:58Z

Taking a quick look at the screenshots, if the two hover texts are for the different layers, they look the same to me; it appears somehow the two copies got added with the same name some how. I'll try to reproduce and see how the data label is being applied; I could have sworn when a data label already exists, Jdaviz automatically appends a (1) to the end of it

camipacifici · 2024-09-16T18:46:32Z

Careful, they are not the same. One is gsd04 and one is gsd12_sect32.

duytnguyendtn · 2024-09-16T18:47:58Z

Hmm, no idea then! I'll poke around and see if I find anything

kecnry · 2024-09-26T18:56:41Z

jdaviz/configs/default/plugins/virtual_observatory/vo_plugin.py

+from pyvo.utils import vocabularies
+from pyvo import registry
+from pyvo.dal.exceptions import DALFormatError


is pyvo considered a dependency (should be added to pyproject.toml) or optional (try/except on the import and then set a disabled message on the plugin instructing to install pyvo in order to use the plugin)?

Either way, should merging this be blocked waiting for the 1.6 release of pyvo or is it partially functional without?

kecnry · 2024-09-27T17:43:04Z

jdaviz/components/plugin_table.vue

@@ -39,7 +39,7 @@
        </template>
        </v-select>
      </div>
-      <div style="line-height: 64px; width=32px" class="only-show-in-tray">
+      <div style="line-height: 64px; width:32px" class="only-show-in-tray">


good catch! 😬

Suggested change

<div style="line-height: 64px; width:32px" class="only-show-in-tray">

<div style="line-height: 64px; width: 32px" class="only-show-in-tray">

kecnry · 2024-09-27T18:13:43Z

jdaviz/configs/default/plugins/virtual_observatory/vo_plugin.py

+        self.wavebands = [
+            w.lower() for w in vocabularies.get_vocabulary("messenger")["terms"]
+        ]
+        self.waveband_selected = None


can this use a SelectPluginComponent?

kecnry · 2024-09-27T18:14:58Z

jdaviz/configs/default/plugins/virtual_observatory/vo_plugin.py

+        # Check mixin object initialized
+        if hasattr(self, "viewer"):


elsewhere we do the following instead which might help readability (whitespace after this will also need updating):

Suggested change

# Check mixin object initialized

if hasattr(self, "viewer"):

# Check mixin object initialized

if not hasattr(self, "viewer"):

return

kecnry · 2024-09-27T18:16:32Z

jdaviz/configs/default/plugins/virtual_observatory/vo_plugin.py

+            for viewer in self.viewer.viewers:
+                if viewer == self.viewer.selected_obj:
+                    viewer.state.add_callback("zoom_center_x", self.vue_center_on_data)
+                    viewer.state.add_callback("zoom_center_y", self.vue_center_on_data)
+                else:
+                    # If not subscribed anyways, remove_callback should produce a no-op
+                    viewer.state.remove_callback(
+                        "zoom_center_x", self.vue_center_on_data
+                    )
+                    viewer.state.remove_callback(
+                        "zoom_center_y", self.vue_center_on_data
+                    )


could this just use the new/old information in the event instead of looping through everything?

kecnry · 2024-09-27T18:17:21Z

jdaviz/configs/default/plugins/virtual_observatory/vo_plugin.py

+                    viewer.state.remove_callback(
+                        "zoom_center_y", self.vue_center_on_data
+                    )
+            self.vue_center_on_data()


do we want this regardless of coord_follow_viewer_pan? If so, can you please add an inline comment why its here?

kecnry · 2024-09-27T18:25:23Z

jdaviz/configs/default/plugins/virtual_observatory/vo_plugin.py

+        # Reset Table
+        self.table.items = []
+        self._populate_url_only = False
+        self.table.headers_visible = ["Title", "Instrument", "DateObs"]


should this wait until we know it will be successful? Otherwise any exception will leave the table reset (or maybe that makes sense).

kecnry · 2024-09-27T18:25:33Z

jdaviz/configs/default/plugins/virtual_observatory/vo_plugin.py

+        self._populate_url_only = False
+        self.table.headers_visible = ["Title", "Instrument", "DateObs"]
+
+        self.results_loading = True  # Start loading spinner


same as above - could use with_spinner decorator.

kecnry · 2024-09-27T18:25:54Z

jdaviz/configs/default/plugins/virtual_observatory/vo_plugin.py

+            except Exception:
+                try:
+                    # If that didn't work, try parsing it as an object name
+                    coord = SkyCoord.from_name(
+                        self.source, frame=self.coordframe_selected
+                    )
+                except Exception:
+                    raise LookupError(
+                        f"Unable to resolve source coordinates: {self.source}"
+                    )


same as above - are there specific exceptions we can catch?

kecnry · 2024-09-27T18:30:09Z

jdaviz/configs/default/plugins/virtual_observatory/vo_plugin.vue

+          v-model="radius_unit_selected"
+          attach
+          :items="radius_unit_items.map(i => i.label)"
+          label="Unit">


Suggested change

v-model="radius_unit_selected"

attach

:items="radius_unit_items.map(i => i.label)"

label="Unit">

v-model="radius_unit_selected"

attach

:items="radius_unit_items.map(i => i.label)"

label="Unit">

kecnry · 2024-09-27T18:31:01Z

jdaviz/configs/imviz/plugins/orientation/orientation.py

I'm guessing there aren't any actual changes here - can this be removed from the diff?

kecnry

Few more comments from testing the plugin itself - overall this seems to have a lot of edge cases covered and a lot of UI-polish, most of my comments are related to tooltips and using some re-usable components that now exist in jdaviz so that we have consistent styling.

kecnry · 2024-09-30T17:07:12Z

jdaviz/configs/default/plugins/virtual_observatory/vo_plugin.vue

+          <v-btn
+            id="querybtn"
+            @click="query_registry_resources"
+            icon
+            :loading="resources_loading">
+            <v-icon>mdi-refresh</v-icon>
+          </v-btn>


this button could use a tooltip. Is there anytime its useful to trigger this manually since it seems to automatically update when changing waveband?

kecnry · 2024-09-30T17:08:54Z

jdaviz/components/tooltip.vue

+    <div style="border: 1px solid gray;" class="pa-2">
+      <strong>Note:</strong>
+      Surveys which have not implemented coverage information will also be excluded. If you are expecting a survey that doesn't appear, try disabling coverage filtering.
+    </div>`


can this have a max-width or something set? Right now this results in an extremely wide tooltip which might not fit at smaller resolutions, etc.

kecnry · 2024-09-30T17:21:12Z

jdaviz/configs/default/plugins/virtual_observatory/vo_plugin.vue

+          <v-btn
+            id="autocenterbtn"
+            @click="center_on_data"
+            :disabled="viewer_centered"
+            icon>
+            <v-icon>
+              {{ viewer_centered ? 'mdi-crosshairs-gps' : 'mdi-crosshairs' }}
+            </v-icon>
+          </v-btn>


without a label, this would benefit from a tooltip

kecnry · 2024-09-30T17:37:54Z

jdaviz/configs/default/plugins/virtual_observatory/vo_plugin.vue

+    <v-row class="row-no-outside-padding">
+      <v-col>
+        <v-btn
+          block
+          color="primary"
+          :loading="results_loading"
+          :disabled="!all_fields_filled"
+          text
+          @click="query_resource">Query Archive</v-btn>
+      </v-col>
+    </v-row>
+
+    <jupyter-widget :widget="table_widget"></jupyter-widget>
+
+    <v-row class="row-no-outside-padding">
+        <v-col>
+            <v-btn color="primary" :loading="data_loading" text @click="load_selected_data">Load Data</v-btn>
+        </v-col>
+    </v-row>


can these use plugin-action-button which should also provide consistent styling/layout (right now the first button takes the full width due to block while the second does not and is right-aligned)?

kecnry · 2024-09-30T17:39:18Z

jdaviz/configs/default/plugins/virtual_observatory/vo_plugin.vue

+        ></v-select>
+      </v-row>
+
+    <j-plugin-section-header>Common Options</j-plugin-section-header>


is there room in the future for more in this section or will it remain just the search radius? Maybe the header can either be more specific ("search radius/terms") or the section just removed and search criteria lumped in with source selection?

kecnry · 2024-09-30T17:40:30Z

jdaviz/configs/default/plugins/virtual_observatory/vo_plugin.vue

+        <j-tooltip tipid='plugin-vo-filter-coverage'>
+        <span>
+          <v-btn icon @click.stop="resource_filter_coverage = !resource_filter_coverage">
+            <v-icon>mdi-filter{{ resource_filter_coverage ? '' : '-remove' }}</v-icon>
+          </v-btn>
+          Filter by Coverage
+        </span>
+        </j-tooltip>


maybe this should just be a switch instead so that there is no ambiguity about whether its enabled or not.

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

duytnguyendtn force-pushed the vo branch 3 times, most recently from ea36f51 to cd73c0e Compare July 23, 2024 18:13

duytnguyendtn added 25 commits July 24, 2024 10:36

Migrate VO plugin to Jdaviz

a8a458e

Print resource url in empty error

b4c3530

Clarify degree unit on coord tooltip

3b6ffa6

Catch connection error

0d688d9

Allow URL only override

945677c

Automatically fallback table to URL-only if unable to get metadata co…

83fa048

…lumns

Coordinate frame option

abb24ec

Autocenter coords on viewer if data present

bb41ee5

Filter resource querying on coverage at source, adjust userflow to se…

7fffc8f

…lect source before resource, add manual coordinate toggle to viewer dropdown

Coverage filtering toggle

1c67aca

Migrate resource_selected to v-model

bf4a94f

Mark Source and Resource as required fields

6a555f9

Remove redundant manual traitlet sync

9348eef

Set throwaway event args to optional

d580c66

Drop vestigial argument signatures

4e18af2

Set query registry to observe waveband_selected to avoid multiple reg…

76581ca

…istry calls while typing the source name

Add TODO comment

81e6cb1

Black Formatting

ca41b24

First draft documentation

9d007b8

Fix doc references

a3295ce

Fix IVOA hyperlink

09a21bb

Docs: Add mention to source name string input

d22ddd7

Local autocenter test

c0ecc2a

Coverage toggle test

2ca1cbf

Move table population to testable method for local test

a357e83

Warn users about potentially misaligned data if WCS linking isn't ena…

10d712d

…bled

duytnguyendtn added 3 commits August 2, 2024 10:14

Load data errors do not prevent subsequent data products from loading

d4f152a

TEST: Combine load_data error test to main load_data test

Test organization, codestyle

e2e463d

Test that loading spinners turn off when operations are complete

49eefb7

Effectively disable timeout

5a61cc9

duytnguyendtn added 3 commits September 4, 2024 12:24

Recalculate autocoord when linktype changes

c0e9086

Allow float degrees

ea2562e

Add support for radius units

783262a

kecnry reviewed Sep 12, 2024

View reviewed changes

jdaviz/configs/default/plugins/virtual_observatory/vo_plugin.py Outdated Show resolved Hide resolved

duytnguyendtn added 2 commits September 12, 2024 10:54

Use UnitSelectPluginComponent for radius unit

5742480

Add viewer force recenter button

03a00e3

Allow plugin to follow viewer coords thru panning

300f194

duytnguyendtn added 3 commits September 13, 2024 17:04

Don't bother subscribing if we're on Manual

8898c55

Update test method call after renaming

0ff66b1

UI: Change autocenterbtn icon to indicate when coords are centered or…

fd89b64

… not

duytnguyendtn mentioned this pull request Sep 20, 2024

[BUG] Incomputable histogram crashes Jupyter kernel #3199

Open

kecnry reviewed Sep 26, 2024

View reviewed changes

kecnry reviewed Sep 27, 2024

View reviewed changes

kecnry reviewed Sep 30, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Virtual Observatory plugin for Jdaviz #2872

Virtual Observatory plugin for Jdaviz #2872

duytnguyendtn commented May 10, 2024 •

edited

Loading

camipacifici commented Jul 31, 2024

duytnguyendtn commented Jul 31, 2024

camipacifici commented Aug 1, 2024

duytnguyendtn commented Aug 2, 2024

duytnguyendtn commented Aug 7, 2024

duytnguyendtn commented Sep 12, 2024 •

edited

Loading

duytnguyendtn commented Sep 13, 2024

camipacifici commented Sep 16, 2024

duytnguyendtn commented Sep 16, 2024

camipacifici commented Sep 16, 2024

duytnguyendtn commented Sep 16, 2024

kecnry Sep 26, 2024

kecnry Sep 27, 2024

kecnry Sep 27, 2024

kecnry Sep 27, 2024

kecnry Sep 27, 2024

kecnry Sep 27, 2024

kecnry Sep 27, 2024

kecnry Sep 27, 2024

kecnry Sep 27, 2024

kecnry Sep 27, 2024

kecnry Sep 27, 2024

kecnry left a comment

kecnry Sep 30, 2024

kecnry Sep 30, 2024

kecnry Sep 30, 2024

kecnry Sep 30, 2024

kecnry Sep 30, 2024

kecnry Sep 30, 2024

	<div style="line-height: 64px; width:32px" class="only-show-in-tray">
	<div style="line-height: 64px; width: 32px" class="only-show-in-tray">

Virtual Observatory plugin for Jdaviz #2872

Are you sure you want to change the base?

Virtual Observatory plugin for Jdaviz #2872

Conversation

duytnguyendtn commented May 10, 2024 • edited Loading

Description

Change log entry

Checklist for package maintainer(s)

camipacifici commented Jul 31, 2024

duytnguyendtn commented Jul 31, 2024

camipacifici commented Aug 1, 2024

duytnguyendtn commented Aug 2, 2024

duytnguyendtn commented Aug 7, 2024

duytnguyendtn commented Sep 12, 2024 • edited Loading

duytnguyendtn commented Sep 13, 2024

camipacifici commented Sep 16, 2024

duytnguyendtn commented Sep 16, 2024

camipacifici commented Sep 16, 2024

duytnguyendtn commented Sep 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kecnry left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

duytnguyendtn commented May 10, 2024 •

edited

Loading

duytnguyendtn commented Sep 12, 2024 •

edited

Loading