Skip to content

Selection API #1

Draft
pcuste1 wants to merge 4 commits into
developfrom
selection-api
Draft

Selection API #1
pcuste1 wants to merge 4 commits into
developfrom
selection-api

Conversation

@pcuste1
Copy link
Copy Markdown
Owner

@pcuste1 pcuste1 commented Apr 21, 2025

What is changing?

  • Adds new "regionSelected" listener callback. This callback returns the type of the region drawn (circle, poly, or rect) as well as coordinates needed to define each of the regions
  • Adds new "selectionRegion" method that triggers a full selection event for the provided region

Why is this needed?

This API will be used by ipyaladin to give users programmatic access to the selection capabilities of aladin-lite. Users will be able to draw a selection in the aladin-lite widget, export that region, and then import it at a later time. This can aid in repeatability of notebook workflows, the saving of complex selections, and the linking of selection events across different tools to enable scientists needs.

Related PRs

ipyaladin

Future work

  • Expose models from aladin-lite for the different regions so that there is ensure consistency between these two packages
  • Ensure that the math all works out for the different regions. Rectangles in particular might still need a little attention

Copy link
Copy Markdown

@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 added a couple questions in the code. I think a high-level summary of the API changes would be helpful in the PR. I actually don't know offhand where the API docs live.

Comment thread src/js/View.js
}

View.prototype.regionSelection = function(region, callback) {
this.selector.start(region["selection_type"], callback);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

With the callback being given to start(), it's not clear to me what its purpose is. Maybe documentation on the Aladin.prototype.selectRegion() could help with that.

Along the same lines, I wonder if the the next line, this.selector.dispatch() should be waiting for the start() to complete.

view.aladin.removeStatusBarMessage('selector')
};

let api = (params) => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is there a more informative name for api?

y: startCooPix[1],
};

this.dispatch('mouseup', {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

These uses of mouse events to define a selection region are good places to highlight as we discuss the challenges of moving between sky regions and screen coordinates. For these in particular, unless one is zoomed in a centered around the region, it's pretty easy for the sky coordinates needed to not be on the screen.

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.

2 participants