Reset angle to 0 for new subsets

[rosteen]

I don't see a Github issue for this, but it was reported by @cshanahan1:

"If an elliptical or rectangular subset is created, then rotated in the subset plugin, the next created subset of that type will preserve the old orientation rather than creating one with the expected default angle of 0."

Turns out this is because the ROI gets cached on the glue-jupyter toolbar tool itself if it was still active when the angle was changed. This PR checks the active tool for all viewers when "Create New" is selected to see if the active tools have an _roi with a theta attribute and if so, sets it to 0.

[rosteen]

Moved back to draft, I realized this fix causes the angle to not update correctly if you go back and select a subset to edit that had a non-zero angle.

[codecov]

Codecov Report

Attention: 47 lines in your changes are missing coverage. Please review.

Comparison is base (3e904a4) 91.47% compared to head (85e9816) 91.52%.
Report is 49 commits behind head on main.

Files	Patch %	Lines
jdaviz/core/template_mixin.py	91.03%	13 Missing ⚠️
jdaviz/core/tools.py	79.54%	9 Missing ⚠️
...nfigs/default/plugins/plot_options/plot_options.py	86.66%	6 Missing ⚠️
...plugins/spectral_extraction/spectral_extraction.py	86.48%	5 Missing ⚠️
...daviz/configs/default/plugins/collapse/collapse.py	86.48%	5 Missing ⚠️
...configs/cubeviz/plugins/moment_maps/moment_maps.py	95.94%	3 Missing ⚠️
jdaviz/configs/specviz/helper.py	0.00%	2 Missing ⚠️
jdaviz/app.py	83.33%	1 Missing ⚠️
jdaviz/configs/specviz/plugins/parsers.py	80.00%	1 Missing ⚠️
jdaviz/core/user_api.py	0.00%	1 Missing ⚠️
... and 1 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2639      +/-   ##
==========================================
+ Coverage   91.47%   91.52%   +0.05%     
==========================================
  Files         160      161       +1     
  Lines       19581    19993     +412     
==========================================
+ Hits        17911    18299     +388     
- Misses       1670     1694      +24     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[rosteen]

@cshanahan1 @gibsongreen I think this is ready for review now, I changed to wiping the ROI from the tool completely rather than updating theta to 0 and it seems to have solved the problem with resetting the previous subsets.

[kecnry]

This doesn't seem to do the trick when "add" selection is used, although does fix the much more common scenarios, so if there isn't an easy workaround, I'd still be in favor of getting this in.

Screen.Recording.2023-12-28.at.12.19.23.PM.mov

[rosteen]

This doesn't seem to do the trick when "add" selection is used, although does fix the much more common scenarios, so if there isn't an easy workaround, I'd still be in favor of getting this in.

I would consider this a separate case, and I'm not even sure if that behavior is desired or not. It seems more reasonable for a user to need to manually edit the rotation back to zero when editing (adding to) an existing subset vs creating a brand new one 🤔

Edit: I'll think about this a little more before committing to handling it here or not, it might be easier than my initial guess.

[kecnry]

I'm not even sure if that behavior is desired or not. It seems more reasonable for a user to need to manually edit the rotation back to zero when editing (adding to) an existing subset vs creating a brand new one

But each sub-component is assigned its own angle, so I'd definitely expect a new sub-component to act as drawn and show an angle of zero by default rather than adopting the angle of the last (or is it most-recently edited 🤷) existing sub-component.

Screen.Recording.2023-12-28.at.12.25.48.PM.mov

[haticekaratay]

I have tested several workflows and confirmed that they resolved the issue described. I have also observed the behavior mentioned by Kyle when adding a new subset with "add" mode, which adopts the previously added angle. Can you please clarify if the fix of this behavior is within the scope of this effort? If not, I can approve the current solution as it stands. Thanks

[rosteen]

I have tested several workflows and confirmed that they resolved the issue described. I have also observed the behavior mentioned by Kyle when adding a new subset with "add" mode, which adopts the previously added angle. Can you please clarify if the fix of this behavior is within the scope of this effort? If not, I can approve the current solution as it stands. Thanks

I think you can go ahead and approve as it stands, we'll hopefully address that upstream.

[pllim]

Is this test-able in CI?

Also, did you confirm that this did not break "drag and move" or "drag to resize" cases?

[pllim]

If this indeed works as expected in all cases, then I think this patch can go in. If we decide to fix this upstream, we can revert this later. With Tom's limited availability and Derek's plate full, this patch seems trivial and we should not drag it on.

[rosteen]

Also, did you confirm that this did not break "drag and move" or "drag to resize" cases?

In the drag case, the "preview" gray area has 0 angle, but the actual subset when dragged keeps the angle as it should. I was hoping that a proper upstream fix would remedy that, I don't think there's a way around it from the jdaviz side.

[pllim]

Gray not listening to angle is a known issue. If I remember correctly, SME said the fix for that isn't trivial so we decided to live with it.

[rosteen]

I think we'll leave this open for now, and if an upstream fix in glue-jupyter isn't ready by the time we do our next bugfix release, we can merge this then and revert later.

[pllim]

Seems to work. Can't think of an easy way to test in CI; would have to fake some event messages. Should we merge then?

p.s. Found a "hidden feature" (at least I didn't know I could do that): Select existing subset with simple shape, select "add" tool, click on the Subset to drag, drag it. Instead of moving, it creates a composite subset with two of the same shapes but at different locations now. But you can only do that once because the drag tool does not work on composite subset.

[pllim]

I created an issue for the "gray shadow" at glue-viz/bqplot-image-gl#106

[dhomeier]

FYI I have pushed a reset of angle and other properties on deactivate of the selection tools in glue-viz/glue-jupyter#420. I don't think more aggressive resetting can be done within glue-jupyter.

[rosteen]

This is getting fixed upstream, I'm going to close this PR.