Display sky coordinates in subset plugin when WCS linked #2553
Conversation
|
Working with a regular circle and switching between link types seems to work. I say "seems" because I can't remember all the conversations that took place over the image rotation PR about what we expect to happen when the link type changes like that. I tried to use an annulus and when I tried to update one of the values, the subset disappeared. Otherwise I think this is on the right track! I'll look through the code next. Also, it would be good to get the code style checks fixed so we can see if this breaks CI anywhere. |
| if self.subset_selected != self.subset_select.default_text: | ||
| self._get_subset_definition(*args) |
There was a problem hiding this comment.
self.subset_selected can be a list in multiselect mode, so this may need some more checks.
| @@ -139,13 +160,18 @@ def _unpack_get_subsets_for_ui(self): | |||
| Convert what app.get_subsets returns into something the UI of this plugin | |||
| can display. | |||
| """ | |||
|
|
|||
| self.hub.broadcast(SnackbarMessage(f'in _unpack_subsets_for_ui', color='error', sender=self)) | |||
There was a problem hiding this comment.
Just a reminder to remove this when you take this PR out of draft mode.
| x, y = subset_state.roi.center() | ||
| r = subset_state.roi.radius | ||
| if self.display_sky_coordinates: | ||
| sky_region = spec['sky_region'] |
There was a problem hiding this comment.
Maybe sky_region does not know how to handle annulus types?
|
Note to self: When we get back to this after #2179 is merged, need to see if Might also have to rewrite these lines in the Imviz example notebooks so that they work in WCS linking mode (they still work when pixel linked): mask = regions['Subset 1'].to_mask(mode='subpixels')
data = imviz.get_data('acs_47tuc_1[SCI,1]')
plt.imshow(mask.to_image(data.shape), origin='lower') |
|
@cshanahan1 remind me, what was the holdup/problem with the rectangular subsets? Everything else seems to be working so far in my testing. |
i fixed that. theres some very strange, low level issue with updating theta on elliptical subsets but that shouldn't hold up this pr. I am trying to find a few mins (just kidding, probably an hour :( ) to rebase this, thanks for testing and I will do that as soon as i can!! |
|
Ah, gotcha, alright I'll hold off on testing more then until you update the PR (the rectangle handling is stuff you haven't pushed up yet, right?). |
|
nope thats in there already! |
|
Oh, well, in that case it doesn't seem to be working for me. I'll grab a screen recording in a minute. |
|
There's no Screen.Recording.2023-11-09.at.1.31.03.PM.mov |
|
@rosteen im so sorry i think the wrong PR is linked in the jira ticket! The one I've been updating is on Brett's PR bmorris3#19. I'll close this one and update the jira ticket |
|
Closing, bmorris3#19 contains this work |
Draft PR to add sky coordinates in subset plugin when wcs linked