Disable astrowidgets pixel marking when WCS linked (PR to PR 14 to PR 2179) #5
Conversation
* instead, show snackbar when layers are removed when switching to WCS-linked
* other orientation layers can be deleted. If the currently selected orientation is deleted, that viewer will default back to Default orientation
generalization still needs to be made for the multi-viewer case
|
does this address whether a given entry should or should not be in the data menu (or removed when changing from pixel->WCS linking)? Or is the plan to put that directly in bmorris3#14 and rebase this on top of that once that is merged? |
Data menu is irrelevant here. If it is not allowed, it won't be loaded in the first place. Once loaded, changing the linking type is disabled until all markers are removed. The latter is an existing behavior and if we end up moving to using a list of regions, also irrelevant. |
|
So is bmorris3#14 (comment) no longer a concern? If you add another viewer is the "+" button enabled/disabled as you'd expect? |
I can't keep track of what happened but Imviz now let me load the tables. Did you change the logic or something? |
kecnry
force-pushed
the
wcs-only-data-menu-fixes
branch
from
d1ab573 to
c33b10e
Compare
|
@kecnry , I see that you force pushed and now my diff also has your other changes. Is this intentional? |
|
I thought the discussed plan was to get bmorris3#14 in and then re-open your branch on top of that (instead of PR to PR to PR) |
|
Yeah... I didn't think that plan include force pushing to my branch so soon. I thought I would just rebase on top of Brett's branch after your PR is merged, which has not happened yet. |
|
I did not force push to your branch, just the base branch ( |
|
Oh, sorry. I misunderstood the message on GitHub. Anyway, I can figure this out later after your PR is merged. |
|
Gonna close this and open against Brett's branch. |
|
This is now bmorris3#16 |
Description
This pull request is to address adding markers in pixel space breaking when there is rotation layer. Markers added with SkyCoord already satisfies "marks should adjust to a change in orientation layer" (see screenshots below).
This is a PR to:
that is a PR to:
I fixed the one test failure I know is caused by my change here. When I ran the full test suite locally, there were several other test failures that look like they are from different parts of the code, but the PR 2179 branch is also outdated compared to latest
main, so some of those maybe already fixed inmain, so the PR 2179 branch should be rebased first before fixing the tests.Default orientation
N-up E-left
🐱