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: astrowidgets + Ginga #406

Closed
wants to merge 3 commits into from
Closed

Conversation

pllim
Copy link
Contributor

@pllim pllim commented Jan 22, 2021

For JDAT-1192 .

Current thoughts: Despite that was told, I think the jdaviz machinery depends too much on Glue. I am not confident that you can just drop astrowidgets in there without significant refactoring.

TODO

  • Release new astrowidgets version so we can pick it off PyPI. (Released 0.2.0)
  • Investigate error from viewer. (See Error below.) -- Ricky fixed it. Thanks, Ricky!
  • Resolve incompatibility between dev versions of astrowidgets and Ginga (Error redrawing image: 'CanvasRenderer' object has no attribute 'std_order' ejeschke/ginga#925).
  • Investigate why it says data loaded but nothing is showing. Maybe some of the dummy classes need to be filled in with real code but I will need a Glue expert to help here.
  • I still don't see the point of this. The data flow is too complicated: Image -> app (Glue) -> image viewer (astrowidgets/Ginga with lots of hacks) -> Glue internals
    • If you want the interactive portion to be handled by astrowidgets, how do you send the data back and forth? Right now, the data gets sent to Glue and then that's it. I don't know how to get it back.

Error

Traceback (outdated)
../jdaviz/core/helpers.py in load_data(self, data, parser_reference, **kwargs)
     35 
     36     def load_data(self, data, parser_reference=None, **kwargs):
---> 37         self.app.load_data(data, parser_reference=parser_reference, **kwargs)

.../jdaviz/app.py in load_data(self, file_obj, parser_reference, **kwargs)
    295             # If the parser returns something other than known, assume it's
    296             #  a message we want to make the user aware of.
--> 297             msg = parser(self, file_obj, **kwargs)
    298 
    299             if msg is not None:

.../jdaviz/configs/imviz/plugins/parsers.py in imviz_image_parser(app, data, data_label, show_in_viewer)
     24         raise FileNotFoundError(f"No such file: {path}")
     25 
---> 26     app.add_data(data, data_label)
     27     if show_in_viewer:
     28         app.add_data_to_viewer("image-viewer", data_label)

.../jdaviz/app.py in add_data(self, data, data_label)
    570         # Include the data in the data collection
    571         data_label = data_label or "New Data"
--> 572         self.data_collection[data_label] = data
    573 
    574         # Send out a toast message

.../glue/core/data_collection.py in __setitem__(self, key, data)
    380         if not isinstance(data, Data):
    381 
--> 382             handler, preferred = data_translator.get_handler_for(data)
    383 
    384             data = handler.to_data(data)

.../glue/config.py in get_handler_for(self, data_or_class)
    564                                 "type Data to {0}".format(data_or_class.__name__))
    565             else:
--> 566                 raise TypeError("Could not find a class to translate objects of "
    567                                 "type {0} to Data".format(data_or_class.__class__.__name__))
    568         return handler, preferred

TypeError: Could not find a class to translate objects of type ndarray to Data

@pllim pllim added the imviz label Jan 22, 2021
@pllim
Copy link
Contributor Author

pllim commented Jan 22, 2021

Also, this seems a little convoluted (Ginga -> astrowidgets -> glue -> glue-jupyter -> notebook) when we can simply do this (Ginga -> astrowidgets -> notebook).

Existing notebook solution that already works: https://github.com/astropy/astrowidgets/tree/master/example_notebooks

Existing standalone solution that already works: https://ginga.readthedocs.io/en/stable/manual/concepts.html (Qt or GTK backend)

@rosteen
Copy link
Collaborator

rosteen commented Jan 25, 2021

Replacing data = fits.getdata(path) with data = CCDData.read(path) gets past this error (since there is a glue-astronomy translator for CCDData, see https://github.com/glue-viz/glue-astronomy/blob/master/glue_astronomy/translators/ccddata.py), and it then hits another error:

~/projects/jdaviz/jdaviz/app.py in _update_selected_data_items(self, viewer_id, selected_items)
    906             [data] = [x for x in self.data_collection if x.label == label]
    907 
--> 908             viewer.add_data(data)
    909 
    910             add_data_message = AddDataMessage(data, viewer,

AttributeError: 'ImvizImageView' object has no attribute 'add_data'

I think that this is simple to implement.

@pllim pllim mentioned this pull request Jan 27, 2021
6 tasks
pllim added a commit to pllim/jdaviz that referenced this pull request Jan 27, 2021
@pllim
Copy link
Contributor Author

pllim commented Jan 28, 2021

I cannot get the data to show. There are no errors. Data is just lost in translation. 🤷

pllim added a commit to pllim/jdaviz that referenced this pull request Jan 28, 2021
Base automatically changed from master to main January 29, 2021 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants