Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JDAT-1220: Regions Support #518

Closed
6 of 9 tasks
pllim opened this issue Apr 13, 2021 · 8 comments
Closed
6 of 9 tasks

JDAT-1220: Regions Support #518

pllim opened this issue Apr 13, 2021 · 8 comments

Comments

@pllim
Copy link
Contributor

pllim commented Apr 13, 2021

Ability to draw fractional pixel regions of various shapes that can be exported as Astropy regions.

UI/UX: https://www.stsci.edu/~jkotler/Imviz/#id=xvaaay&p=region_subset_workflow&g=1

  1. Menu to select various shapes (as in existing jdaviz viewers)
  • Button for Circles
  • Button for Rectangles
  1. Integrate subsets pulldown with region selection (see mockup). Shouldn't have to "Create new" to make a new region. Clicking and dragging with region tool selected creates new region and adds it to the list of subsets.

OUT OF SCOPE: Getting slit info for an instrument from header, and so on. This issue only covers hand-drawn slits.

https://jira.stsci.edu/browse/JDAT-1220

@pllim pllim added this to the Imviz 1.0 milestone Apr 13, 2021
@pllim pllim self-assigned this May 11, 2021
@pllim

This comment has been minimized.

@eteq
Copy link
Contributor

eteq commented May 12, 2021

After some discussion, I'm now thinking that " Integrate subsets pulldown with region selection" is better done as a separate issue, since it's a much bigger project than the other parts of this.

@eteq
Copy link
Contributor

eteq commented May 13, 2021

Oh and to clarify a bit more on the "Create new" item: the current behavior is that if you make a new selection it creates a region the first time, but after that it "moves" the region. That bullet point (I think originating from @orifox ?) was to instead make it create a new region every time. @pllim and I discussed out-of-band that we think the current behavior is still what we want as default. But there is value in an optional mode where instead of glue changing the current subset/region to the one newly created, it stays in "create new" mode. We can discuss later if we really want that to be default for imviz.

@astrofrog
Copy link
Collaborator

The place where the behavior is controlled is in the EditSubsetMode class - to create a subset every time one would need to do: glue-viz/glue#2192 - of course we shouldn't merge that PR, but just to point out where the code needs to be changed. Maybe we could make it a (in glue) non-default option to have this behavior, and potentially have a way to toggle that option from the jdaviz UI. Here's what it looks like with that change:

Peek 2021-05-13 16-39

In the above, as long as Create New is selected, it stays on that. If the user selects to edit a certain subset, it will stick to the editing mode.

@eteq
Copy link
Contributor

eteq commented May 18, 2021

After some out-of-band discussion, I think we've settled on the idea that the initial implementation should be:

  • A panel that handles rotation angle and pixel-level centering. That makes the workload for the rectangle easier since no UI changes are needed. We can consider a fancy rotation thingie later if it seems like it's useful, but not in this issue.
  • That panel can later also do things like altering the region properties, etc. It should probably be a jdaviz plugin to start with, but it might/should be baked into glupyter at some point, and we can decide then whether we want to keep it as a "plugin" or not.

@eteq
Copy link
Contributor

eteq commented May 18, 2021

Oh and re: #518 (comment) - I think that's the right behavior for the "create new" item, but not on "drag" of an existing region - rather it should be when the user lets up on the mouse button and draws a new region.

If the "drag" process currently has to do that when it's in the create new mode, I can live with that as a temporary measure, but we should make an issue to fix that at some point since I think to a user it's pretty unintuitive for clicking and dragging to create a bunch of new regions.

@pllim
Copy link
Contributor Author

pllim commented May 18, 2021

pretty unintuitive for clicking and dragging to create a bunch of new regions

Perhaps, but would be very convenient to clone a bunch of shapes to make this possible:

download

@pllim
Copy link
Contributor Author

pllim commented May 21, 2021

At the standup today, @duytnguyendtn and I have agreed to break this into smaller tickets. I will update the issue above and "archive" this when this is done. FYI.

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

No branches or pull requests

3 participants