enable coords-info and markers plugin #22
Conversation
* requires upstream changes to generalize markers plugin
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #22 +/- ##
==========================================
+ Coverage 83.54% 85.30% +1.75%
==========================================
Files 15 19 +4
Lines 693 803 +110
==========================================
+ Hits 579 685 +106
- Misses 114 118 +4
☔ View full report in Codecov by Sentry. |
|
Basic functionality looks good! Though I'm running into a bug and I can't tell if its' with this PR or with the ephemeris/phase viewer. I have not adjusted any parameters in the plugin, just showed the viewer:
If I kick the period to something else and force the points to redraw, the hoverover seems to be constrained properly |
|
Ok, can you file an issue for that (I personally don't think it should hold up this PR)? @rosteen ran into something similar in the review of the phase-folding plugin, but apparently the fix didn't handle every situation. I'm guessing its actually an upstream bug, but I cannot reproduce it on my machine and it will need more investigation. |
There was a problem hiding this comment.
Functionality works well (with the exception for the above bug) and the code looks as I would expect. Thanks!
@bmorris3 I feel like this PR is straight forward enough. You can decide whether you'd like to still review!
This PR implements and enables both mouseover coordinates and the markers plugin (making use of the generalization in spacetelescope/jdaviz#2213 which has since been released and should be included with the pinned version of jdaviz). Unlike the implementation in specviz, the neareset-point logic is done based on the nearest point in the pixel-space, so that highlighting points in eclipses/transits feels more natural.
Screen.Recording.2023-05-26.at.9.26.32.AM.mov