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

enable clone viewer for image/TPF viewer #101

Merged
merged 4 commits into from
Apr 4, 2024

Conversation

kecnry
Copy link
Member

@kecnry kecnry commented Mar 27, 2024

This pull request enables the clone viewer functionality for the TPF/image viewers as well as the ability to create a new blank cube viewer (so long as cube-like data exists in the data collection), and in doing so also:

  • expands clone viewer logic to adopt layer-level plot options
  • removes duplicated clone viewer logic

NOTE: this should be tested with jdaviz main (3.9) and is expected to fail tests on CI until rebased on top of #68. Tests do pass locally for me and we'll ensure they pass before merging #82 into main.

Screen.Recording.2024-03-27.at.3.10.44.PM.mov

@kecnry kecnry mentioned this pull request Mar 27, 2024
1 task
@@ -24,13 +29,18 @@ def _rebuild_available_viewers(self, *args):
if self.app._jdaviz_helper is not None:
phase_viewers = [{'name': f'lcviz-phase-viewer:{e}', 'label': f'flux-vs-phase:{e}'}
for e in self.app._jdaviz_helper.plugins['Ephemeris'].component.choices] # noqa
if self.app._jdaviz_helper._has_cube_data:
Copy link
Member Author

Choose a reason for hiding this comment

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

right now this enables creating an "image" viewer if there is any cube-like data in the app. In the future, we may want to change this to allow image-like data, but that is not yet supported at the parser level.

Comment on lines -256 to -282
def clone_viewer(self):
name = self.jdaviz_helper._get_clone_viewer_reference(self.reference)

self.jdaviz_app._on_new_viewer(NewViewerMessage(self.__class__,
data=None,
sender=self.jdaviz_app),
vid=name, name=name)

this_viewer_item = self.jdaviz_app._get_viewer_item(self.reference)
this_state = self.state.as_dict()
for data in self.jdaviz_app.data_collection:
data_id = self.jdaviz_app._data_id_from_label(data.label)
visible = this_viewer_item['selected_data_items'].get(data_id, 'hidden')
self.jdaviz_app.set_data_visibility(name, data.label, visible == 'visible')
# TODO: don't revert color when adding same data to a new viewer
# (same happens when creating a phase-viewer from ephemeris plugin)

new_viewer = self.jdaviz_app.get_viewer(name)
if hasattr(self, 'ephemeris_component'):
new_viewer._ephemeris_component = self._ephemeris_component
for k, v in this_state.items():
if k in ('layers',):
continue
setattr(new_viewer.state, k, v)

return new_viewer.user_api

Copy link
Member Author

Choose a reason for hiding this comment

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

this was duplicated logic that was somehow introduced when refactoring to use the mixin for clone viewer support

@kecnry kecnry marked this pull request as ready for review March 27, 2024 19:19
@kecnry kecnry requested a review from bmorris3 March 27, 2024 19:19
Copy link
Contributor

@bmorris3 bmorris3 left a comment

Choose a reason for hiding this comment

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

This looks good. I know this comment isn't specific to this PR, but what is the viewer icon meant to represent? Is it a histogram?

@kecnry
Copy link
Member Author

kecnry commented Apr 4, 2024

but what is the viewer icon meant to represent? Is it a histogram?

yeeeeaaah, I think its just supposed to generically represent "graph" or "plot". I agree it isn't ideal, but that is defined in jdaviz and adopted here 🤷‍♂️

@kecnry kecnry merged commit 2a8a2f9 into spacetelescope:feature-tpf Apr 4, 2024
4 of 9 checks passed
@kecnry kecnry deleted the tpf-clone-viewer branch April 4, 2024 14:12
kecnry added a commit that referenced this pull request Apr 5, 2024
* Adding TPF translator and parser (#75)
* TPF viewer (#81)
* make use of upstream refactor to override indices in lcviz (#83)
* fix creating phase-viewer when TPF is loaded (#86)
* Time Selector (adapted version of cubeviz's slice) plugin (#85)
* enable clone viewer for image/TPF viewer (#101)

---------

Co-authored-by: Brett M. Morris <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants