Skip to content

CST-210: Selection API Discussion #22

Merged
tomdonaldson merged 10 commits into
spacetelescope:mainfrom
pcuste1:CST-210
May 9, 2025
Merged

CST-210: Selection API Discussion #22
tomdonaldson merged 10 commits into
spacetelescope:mainfrom
pcuste1:CST-210

Conversation

@pcuste1
Copy link
Copy Markdown
Contributor

@pcuste1 pcuste1 commented Apr 15, 2025

What is changing?

Adding a document that outlines the selection API discussion we wish to have with the CDS team.

Related Tickets

https://jira.stsci.edu/browse/CST-210

@pcuste1 pcuste1 changed the title CST-121: Selection API Discussion CST-210: Selection API Discussion Apr 15, 2025

- be able to remove the current selection using a programmatic API

## 1.3. Example Use Cases
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add an example use case for the region selection method and listener

Copy link
Copy Markdown
Collaborator

@tomdonaldson tomdonaldson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @pcuste1 very much for this write up! It is extremely helpful. I've added a few comments and questions to continue the discussion.


- several motivating use cases.

## 1.1. Design Principles
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section is really helpful!

Comment thread docs/discussion/selection-api.md Outdated
- access the list of currently selected sources using a programmatic
API 

- access the region of currently selected sources, if one was used to
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am seeing two possible interpretations of regions handling here. Both have pros and cons.

Are we accessing a region object here, or a list of sources that were selected by a region at some point?

The former allows a variety of robust options including editing a region, but also ties us to some usability questions about how dynamic the implied list of selected sources is as new catalogs or other selection constraints are added.

The latter is no doubt simpler to implement since it does a one time selection/filtering of the objects when the region is defined and doesn't need to include the region in future logic. This makes region selection more analogous to other selection constraints like selecting by keys or selection function, since they offer a one-time selection action. I wonder if some hybrid approach could be considered that would allow us to persist one or more user-defined regions, but allow the user to apply those as selection filters when they want. This would be very straightforward for a Python API, but offering it graphically would be more of a UX challenge.

Copy link
Copy Markdown
Contributor Author

@pcuste1 pcuste1 May 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we accessing a region object here, or a list of sources that were selected by a region at some point?

My intention with this was to access a region object, but I think that providing both methods of analyzing the selection state are valid to support.

To use the search engine metaphor I've grown fond of, I think of the region object as the search query and the list of sources as the results of that search.

Represented graphically, I imagine something like the following table might be useful

selection type coordinates objects
circle <CircleSkyRegion(center=<SkyCoord (ICRS): (ra, dec) in deg (346.62236873, -5.04139925)>, radius=0.004999999895450809 deg)> 100 sources
polygon <PolygonSkyRegion(vertices=<SkyCoord (ICRS): (ra, dec) in deg [(346.62579767, -5.03805622), (346.620253 , -5.03660274), (346.6165322 , -5.0408905 ), (346.62382787, -5.04539634), (346.62791342, -5.03936434)]>)> 50 source 1 footprint

If we want to get real fancy with it, we could even include a thumbnail screenshot of what the selected region looked like against the sky background

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is definitely a good question that we should make sure to hit during our discussion

Comment thread docs/discussion/selection-api.md Outdated

- select all sources within a defined region using a programmatic API

- select a footprint using a programmatic API 
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a general comment for all the footprint API things here, we need to support selection of multiple footprints. Whether we move more towards a model of unifying the handling of footprints and catalog objects is TBD, but conceptually I think the handling should be fairly similar.

Comment thread docs/discussion/selection-api.md Outdated
with a redshift within a defined range. The query might look like the
following
```
aladin.selectSources(match_func: lambda source: source.rshift > 1 and
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an interesting and powerful approach.

Comment thread docs/discussion/selection-api.md Outdated
```
def GetSelection() -> Table
```
Returns an Astropy table containing the currently selected catalog
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should spell out in a little more detail how we handle the case of multiple catalog (or footprint) layers. The current behavior in Aladin Lite seems intuitive in that a selection action applies to all layers (at least visible ones, so visibility may be another complication for the programmatic API). Then GetSelections returns a list-like structure (one item per layer) of lists/tables describing the selected objects for each layer. This would apply to the Python and JavaScript APIs.

```
Returns the list of currently selected source objects.

#### GetSelectedRegion
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related to the previous discussion about how regions are handled/maintained, maybe one possibility is that Aladin Lite doesn't maintain any persistence of regions, but just provides the building blocks for creating regions, leaving the management of persistent regions and applying them to selections to the Python layer.

Copy link
Copy Markdown
Contributor Author

@pcuste1 pcuste1 May 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's pretty much how the demo implementation I've provided functions! I added a "regionSelected" callback to Aaldin-lite that returns the building blocks for the region that was selected, and the ipyaladin demo uses those to generate and store astropy regions

Comment thread docs/discussion/selection-api.md Outdated
* @param sources: List[Source]
* @param keys: List[str]
*/
function SelectSources(sources, keys)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The details of how this would look are not clear to me, and they may not be very important yet, but an example of how it might look could be helpful.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is what an example implementation might look like. I can add a link to the doc as well


## 3.4. Misc

#### RemoveSelection
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this meant to remove all selections including footprints and sources with possible multiple layers?

Copy link
Copy Markdown
Contributor Author

@pcuste1 pcuste1 May 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I intended for this to be a pass through to the unselectObjects method in aladin-lite. My understanding is that is unselects both footprints and sources.

@pcuste1 pcuste1 requested a review from tomdonaldson May 8, 2025 16:03
Copy link
Copy Markdown
Collaborator

@tomdonaldson tomdonaldson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mentioned some minor typos, and also brought up a couple new questions. Unless answers are readily available, I suggest we just add them in some form to the discussion topics.

Comment thread docs/discussion/selection-api.md Outdated

| **Parameter** | **Type** | **Description** |
| ------------- | --------------- | ---------------------------------------------------- |
| footprints | List[Footprint] | The list of footprints the user would like to select |
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if string matching on STC strings will be sufficient here for identifying footprints or whether we need to adopt some sort of key matching like we did for sources. I think it will be fairly common to have multiple identical footprints that correspond to different observations of the same part of the sky. Often one would want to select all such footprints (and with graphical selections, one would select all whether one wanted to or not), but I'm not sure we can assume that. It's at least worth discussing as part of discussing footprint selections in general.

On a related note, like with catalogs we will be able to have multiple layers containing footprints. I wonder if there will be cases where we want to be specific about which layer(s) we're selecting from in a call like this.

/**
* @param footprints: List[Footprint]
*/
function SelectFootprints(footprints)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The question above in the python API about how to identify footprints probably applies here too.

Comment thread docs/discussion/selection-api.md Outdated
Comment thread docs/discussion/selection-api.md Outdated
Comment thread docs/discussion/selection-api.md Outdated
Comment thread docs/discussion/selection-api.md Outdated
@pcuste1 pcuste1 marked this pull request as ready for review May 9, 2025 18:55
Copy link
Copy Markdown
Collaborator

@tomdonaldson tomdonaldson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @pcuste1! This all looks good.

@tomdonaldson tomdonaldson merged commit 677735b into spacetelescope:main May 9, 2025
9 checks passed
@codecov
Copy link
Copy Markdown

codecov Bot commented May 9, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 0.00%. Comparing base (e9667df) to head (5a5b22c).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##              main   #22        +/-   ##
==========================================
- Coverage   100.00%     0   -100.00%     
==========================================
  Files            3     0         -3     
  Lines           11     0        -11     
==========================================
- Hits            11     0        -11     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants