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

Refactor coordinates info panel to reduce code duplication #1645

Closed
Jdaviz-Triage-Bot opened this issue Sep 12, 2022 · 0 comments · Fixed by #1976
Closed

Refactor coordinates info panel to reduce code duplication #1645

Jdaviz-Triage-Bot opened this issue Sep 12, 2022 · 0 comments · Fixed by #1976

Comments

@Jdaviz-Triage-Bot
Copy link

Jdaviz-Triage-Bot commented Sep 12, 2022

Reporter: pllim

Image viewers across Jdaviz, namely, Cubeviz, Imviz, and Specviz2d, have coordinates info panel but with slight variations to handle different needs. They all share the same plugin:

https://github.com/spacetelescope/jdaviz/tree/main/jdaviz/configs/imviz/plugins/coords_info

However, the calls to that plugin in the individual viewers were implemented differently. Most of the code were duplicated with variations. @kecnry said there is a need for a refactor so all viewers can use the same code (e.g., inherit from some mixin class).

Keep in mind that all the vizes have slightly different use cases:

  • Imviz (original implementation): 2D images that can be dithered but linked by WCS. It needs to support any number of arbitrary 2D images setup (some can have WCS, some not, and they can be each linked differently back to reference data in the same session).
  • Cubeviz: Primary data shown is a slice of 3D cube. But the panel must also be able to handle generated 2D products like moment maps.
  • Specviz2d: Data is 2D but it is 2D spectrum that may or may not have WCS. Currently sky coordinates are not supported. Not very sure what variations of use cases we need to support here.

🐱


DISCLAIMER: This issue was autocreated by the Jdaviz Issue Creation Bot on behalf of the reporter. If any information is incorrect, please contact Duy Nguyen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants