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

Imviz: Subset control plugin #729

Closed

Conversation

pllim
Copy link
Contributor

@pllim pllim commented Jul 14, 2021

New plugin to control Subset angle and position. Fix #634

  • Wait for upstream fixes by @astrofrog
    • glue-core
    • bqplot
  • Implement a working UI
    • See TODO notes in the code.
  • Don't think there is a way to test this in CI?

Screenshot 2021-07-15 175456

Note: There is a noticeable lag when the "Subset" drop-down list is first populated when images are linked. I think it is related to #723 and is out of scope here.

@pllim pllim added this to the Imviz 1.0 milestone Jul 14, 2021
@github-actions github-actions bot added the imviz label Jul 14, 2021
@pllim pllim force-pushed the you-spin-me-right-round-right-round branch from 7434300 to 3d387f3 Compare July 15, 2021 22:04
# TODO: Using echo.delay_callback breaks the call too.
cur_roi = self._all_subsets[self._selected_subset_label]
try:
cur_roi.move_to(self.new_subset_x, self.new_subset_y)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@astrofrog , this does not give me any Exception but it also does not move the Subset. I cannot tell if I am doing this wrong or this is part of your upcoming upstream fix along with the angle support.

@codecov
Copy link

codecov bot commented Jul 15, 2021

Codecov Report

Merging #729 (3d387f3) into main (92ce20b) will decrease coverage by 0.20%.
The diff coverage is 48.38%.

❗ Current head 3d387f3 differs from pull request most recent head 5120311. Consider uploading reports for the commit 5120311 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main     #729      +/-   ##
==========================================
- Coverage   62.83%   62.62%   -0.21%     
==========================================
  Files          65       67       +2     
  Lines        4286     4348      +62     
==========================================
+ Hits         2693     2723      +30     
- Misses       1593     1625      +32     
Impacted Files Coverage Δ
...igs/imviz/plugins/subset_control/subset_control.py 46.66% <46.66%> (ø)
jdaviz/configs/imviz/plugins/__init__.py 100.00% <100.00%> (ø)
...z/configs/imviz/plugins/subset_control/__init__.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 92ce20b...5120311. Read the comment docs.

@pllim pllim removed this from the Imviz 1.0 milestone Sep 14, 2021
@pllim
Copy link
Contributor Author

pllim commented Sep 14, 2021

PO agreed that this is no longer MVP.

@pllim
Copy link
Contributor Author

pllim commented Sep 16, 2021

p.s. I fantasized about maybe at least get this in only for ellipse for 2.0 but it appears to not be possible anyway because glue-astronomy translator does not take the angle and glue core does not support rotated ellipse. I thought it did. Ah, well.

https://github.com/glue-viz/glue-astronomy/blob/5b8a32c4527b7d15f6cf1405687f52c65f4983fa/glue_astronomy/translators/regions.py#L85

https://github.com/glue-viz/glue/blob/ed71979f8e0e41f993a2363b3b5a8f8c3167a130/glue/core/roi.py#L479-L480

@PatrickOgle
Copy link
Contributor

PatrickOgle commented Oct 1, 2021

A user asked for a similar capability in Cubeviz during Jwebbinar 8. Paraphrased: " I want to specify the centers of the circular subsets by entering their exact values rather than using the cursor". The Subset editor should also have the capability to create a new subset as well as edit an existing one.

@pllim
Copy link
Contributor Author

pllim commented Mar 31, 2022

I think discussions has gone beyond this to something else.

@pllim pllim closed this Mar 31, 2022
@pllim pllim deleted the you-spin-me-right-round-right-round branch March 31, 2022 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JDAT-1484: Imviz region support: Plugin to rotate and center shapes
3 participants