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

jdaviz 3.10 updates #105

Merged
merged 12 commits into from
Jun 11, 2024
Merged

jdaviz 3.10 updates #105

merged 12 commits into from
Jun 11, 2024

Conversation

kecnry
Copy link
Member

@kecnry kecnry commented Apr 15, 2024

TODO:

  • fix loading TPF images
  • fix initial slice in time viewer to be middle of light curve

Waiting for: jdaviz 3.10.2 (update pin in pyproject once released)

Closes #110

@kecnry kecnry added the waiting-on-jdaviz-release PR requires a release from jdaviz before updating the pin label Apr 15, 2024
Copy link

codecov bot commented Apr 15, 2024

Codecov Report

Attention: Patch coverage is 71.42857% with 10 lines in your changes missing coverage. Please review.

Project coverage is 91.23%. Comparing base (7b78df5) to head (a9905cc).
Report is 35 commits behind head on main.

Files with missing lines Patch % Lines
lcviz/helper.py 69.56% 7 Missing ⚠️
lcviz/plugins/coords_info/coords_info.py 40.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #105      +/-   ##
==========================================
- Coverage   93.92%   91.23%   -2.70%     
==========================================
  Files          37       41       +4     
  Lines        1598     2054     +456     
==========================================
+ Hits         1501     1874     +373     
- Misses         97      180      +83     

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

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.

Looks like the upstream slice and display unit changes aren't playing well together:

---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
Cell In[1], line 12
     10 # Load the light curve into LCviz:
     11 lcviz = LCviz()
---> 12 lcviz.load_data(lc_q9)
     13 lcviz.show()

File ~/git/lcviz/lcviz/helper.py:119, in LCviz.load_data(self, data, data_label)
    101 def load_data(self, data, data_label=None):
    102     """
    103     Load data into LCviz.
    104 
   (...)
    117         appended for clarity.
    118     """
--> 119     super().load_data(
    120         data=data,
    121         parser_reference='light_curve_parser',
    122         data_label=data_label
    123     )

File ~/git/jdaviz/jdaviz/core/helpers.py:110, in ConfigHelper.load_data(self, data, data_label, parser_reference, **kwargs)
    108 if data_label:
    109     kwargs['data_label'] = data_label
--> 110 self.app.load_data(data, parser_reference=parser_reference, **kwargs)

File ~/git/jdaviz/jdaviz/app.py:886, in Application.load_data(self, file_obj, parser_reference, **kwargs)
    883         parser = data_parser_registry.members.get(data_parser)
    885 if parser is not None:
--> 886     parser(self, file_obj, **kwargs)
    887 else:
    888     self._application_handler.load_data(file_obj)

File ~/git/lcviz/lcviz/parsers.py:88, in light_curve_parser(app, file_obj, data_label, show_in_viewer, **kwargs)
     86 for viewer_id, viewer in app._viewer_store.items():
     87     if isinstance(viewer, (TimeScatterView, PhaseScatterView)):
---> 88         app.add_data_to_viewer(viewer_id, new_data_label)
     90 # add to any known phase viewers
     91 ephem_plugin = app._jdaviz_helper.plugins.get('Ephemeris', None)

File ~/git/jdaviz/jdaviz/app.py:1605, in Application.add_data_to_viewer(self, viewer_reference, data_label, visible, clear_other_data)
   1602 if viewer_item is None:
   1603     raise ValueError(f"Could not identify viewer with reference {viewer_reference}")
-> 1605 self.set_data_visibility(viewer_item, data_label, visible=visible, replace=clear_other_data)

File ~/git/jdaviz/jdaviz/app.py:2146, in Application.set_data_visibility(self, viewer_reference, data_label, visible, replace)
   2144 if color is None:
   2145     color = viewer.color_cycler()
-> 2146 viewer.add_data(data, percentile=95, color=color)
   2148 # Specviz removes the data from collection in viewer.py if flux unit incompatible.
   2149 if data_label not in self.data_collection:

File ~/git/lcviz/lcviz/viewers.py:230, in TimeScatterView.add_data(self, data, color, alpha, **layer_state)
    212 def add_data(self, data, color=None, alpha=None, **layer_state):
    213     """
    214     Overrides the base class to handle subset styling defaults.
    215 
   (...)
    228         `True` if successful, `False` otherwise.
    229     """
--> 230     result = super().add_data(data, color, alpha, **layer_state)
    232     for layer in self.layers:
    233         # optionally render as a density map
    234         layer.state.density_map = self.density_map

File ~/miniconda3/envs/jdaviz-only-7/lib/python3.11/site-packages/glue_jupyter/view.py:118, in IPyWidgetView.add_data(self, data, color, alpha, **layer_state)
    114 def add_data(self, data, color=None, alpha=None, **layer_state):
    116     data = validate_data_argument(self._data, data)
--> 118     result = super().add_data(data)
    120     if not result:
    121         return

File ~/git/glue/glue/viewers/common/viewer.py:209, in Viewer.add_data(self, data)
    205     raise IncompatibleDataException("Data not in DataCollection")
    207 # Create layer artist and add to container. First check whether any
    208 # plugins want to make a custom layer artist.
--> 209 layer = get_layer_artist_from_registry(data, self) or self.get_data_layer_artist(data)
    211 if layer is None:
    212     return False

File ~/git/glue/glue/viewers/common/viewer.py:240, in Viewer.get_data_layer_artist(self, layer, layer_state)
    239 def get_data_layer_artist(self, layer=None, layer_state=None):
--> 240     return self.get_layer_artist(self._data_artist_cls, layer=layer, layer_state=layer_state)

File ~/miniconda3/envs/jdaviz-only-7/lib/python3.11/site-packages/glue_jupyter/view.py:143, in IPyWidgetView.get_layer_artist(self, cls, layer, layer_state)
    142 def get_layer_artist(self, cls, layer=None, layer_state=None):
--> 143     return cls(self, self.state, layer=layer, layer_state=layer_state)

File ~/miniconda3/envs/jdaviz-only-7/lib/python3.11/site-packages/glue_jupyter/bqplot/scatter/layer_artist.py:81, in BqplotScatterLayerArtist.__init__(self, view, viewer_state, layer_state, layer)
     79 def __init__(self, view, viewer_state, layer_state=None, layer=None):
---> 81     super().__init__(
     82         viewer_state,
     83         layer_state=layer_state,
     84         layer=layer,
     85     )
     87     # Workaround for the fact that the solid line display choice is shown
     88     # as a dashed line.
     89     linestyle_display = {'solid': 'solid',
     90                          'dashed': '– – – – –',
     91                          'dotted': '· · · · · · · ·',
     92                          'dashdot': '– · – · – ·'}

File ~/git/glue/glue/viewers/common/layer_artist.py:29, in LayerArtist.__init__(self, viewer_state, layer_state, layer)
     25 self.state = layer_state or self._layer_state_cls(viewer_state=viewer_state,
     26                                                   layer=self.layer)
     28 if self.state not in self._viewer_state.layers:
---> 29     self._viewer_state.layers.append(self.state)
     31 self.zorder = self.state.zorder
     32 self.visible = self.state.visible

File ~/miniconda3/envs/jdaviz-only-7/lib/python3.11/site-packages/echo/containers.py:52, in CallbackList.append(self, value)
     50 def append(self, value):
     51     super(CallbackList, self).append(self._prepare_add(value))
---> 52     self.notify_all()

File ~/miniconda3/envs/jdaviz-only-7/lib/python3.11/site-packages/echo/containers.py:45, in CallbackList.notify_all(self, *args, **kwargs)
     43 def notify_all(self, *args, **kwargs):
     44     for callback in self.callbacks:
---> 45         callback(*args, **kwargs)

File ~/miniconda3/envs/jdaviz-only-7/lib/python3.11/site-packages/echo/containers.py:166, in dynamic_callback.__call__(self, *args, **kwargs)
    165 def __call__(self, *args, **kwargs):
--> 166     self.function(*args, **kwargs)

File ~/miniconda3/envs/jdaviz-only-7/lib/python3.11/site-packages/echo/containers.py:189, in ListCallbackProperty._default_setter.<locals>.callback(*args, **kwargs)
    188 def callback(*args, **kwargs):
--> 189     self.notify(instance, wrapped_list, wrapped_list)

File ~/miniconda3/envs/jdaviz-only-7/lib/python3.11/site-packages/echo/core.py:125, in CallbackProperty.notify(self, instance, old, new)
    123     return
    124 for cback in self._callbacks.get(instance, []):
--> 125     cback(new)
    126 for cback in self._2arg_callbacks.get(instance, []):
    127     cback(old, new)

File ~/git/glue/glue/viewers/scatter/state.py:195, in ScatterViewerState._layers_changed(self, *args)
    192 if layers_data == layers_data_cache:
    193     return
--> 195 self.x_att_helper.set_multiple_data(self.layers_data)
    196 self.y_att_helper.set_multiple_data(self.layers_data)
    198 self._layers_data_cache = layers_data

File ~/git/glue/glue/core/data_combo_helper.py:322, in ComponentIDComboHelper.set_multiple_data(self, datasets)
    320 for data in unique_data_iter(datasets):
    321     self.append_data(data, refresh=False)
--> 322 self.refresh()

File ~/git/glue/glue/core/data_combo_helper.py:379, in ComponentIDComboHelper.refresh(self, *args)
    376         if len(cids) > 1:
    377             choices += cids
--> 379 self.choices = choices

File ~/git/glue/glue/core/data_combo_helper.py:84, in ComboHelper.choices(self, choices)
     82 @choices.setter
     83 def choices(self, choices):
---> 84     with delay_callback(self.state, self.selection_property):
     85         prop = getattr(type(self.state), self.selection_property)
     86         prop.set_choices(self.state, choices)

File ~/miniconda3/envs/jdaviz-only-7/lib/python3.11/site-packages/echo/core.py:538, in delay_callback.__exit__(self, *args)
    535     self.instance._process_delayed_global_callbacks(resume_props)
    537 for p, args in notifications:
--> 538     p.notify(*args)

File ~/git/glue/glue/utils/matplotlib.py:170, in defer_draw.<locals>.wrapper(*args, **kwargs)
    166 @wraps(func)
    167 def wrapper(*args, **kwargs):
    169     if len(DEFER_DRAW_BACKENDS) == 0:
--> 170         return func(*args, **kwargs)
    172     # Don't recursively defer draws. We just check the first draw_idle
    173     # method since all should be modified in sync.
    174     if isinstance(DEFER_DRAW_BACKENDS[0].draw_idle, DeferredMethod):

File ~/git/glue/glue/viewers/matplotlib/state.py:38, in DeferredDrawSelectionCallbackProperty.notify(self, *args, **kwargs)
     36 @defer_draw
     37 def notify(self, *args, **kwargs):
---> 38     super(DeferredDrawSelectionCallbackProperty, self).notify(*args, **kwargs)

File ~/miniconda3/envs/jdaviz-only-7/lib/python3.11/site-packages/echo/core.py:125, in CallbackProperty.notify(self, instance, old, new)
    123     return
    124 for cback in self._callbacks.get(instance, []):
--> 125     cback(new)
    126 for cback in self._2arg_callbacks.get(instance, []):
    127     cback(old, new)

File ~/git/jdaviz/jdaviz/configs/cubeviz/plugins/slice/slice.py:131, in Slice._initialize_location(self, *args)
    128 # ensure the cache is reset (if previous attempts to initialize failed resulting in an
    129 # empty list as the cache)
    130 viewer._clear_cache('slice_values')
--> 131 slice_values = viewer.slice_values
    132 if len(slice_values):
    133     self.value = slice_values[int(len(slice_values)/2)]

File ~/git/jdaviz/jdaviz/configs/cubeviz/plugins/viewers.py:37, in WithSliceIndicator.slice_values(self)
     33 @property
     34 def slice_values(self):
     35     # NOTE: these are cached at the slice-plugin level
     36     # Retrieve display units
---> 37     slice_display_units = self.jdaviz_app._get_display_unit(
     38         self.slice_display_unit_name
     39     )
     41     def _get_component(layer):
     42         try:
     43             # Retrieve layer data and units

File ~/git/jdaviz/jdaviz/app.py:1298, in Application._get_display_unit(self, axis)
   1295 if self._jdaviz_helper is None or self._jdaviz_helper.plugins.get('Unit Conversion') is None:  # noqa
   1296     # fallback on native units (unit conversion is not enabled)
   1297     if axis == 'spectral':
-> 1298         sv = self.get_viewer(self._jdaviz_helper._default_spectrum_viewer_reference_name)
   1299         return sv.data()[0].spectral_axis.unit
   1300     elif axis == 'flux':

AttributeError: 'LCviz' object has no attribute '_default_spectrum_viewer_reference_name'

@kecnry
Copy link
Member Author

kecnry commented May 31, 2024

@bmorris3 - what jdaviz are you running? This is designed to work with the latest in the v3.10.x branch (but not main).

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.

Try as I might, I couldn't break this. Looks good!

pyproject.toml Outdated Show resolved Hide resolved
from jdaviz.core.helpers import ConfigHelper
from lcviz.viewers import TimeScatterView

__all__ = ['LCviz']


@unit_converter('custom-lcviz')
class UnitConverter:
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's make tickets to add flux unit conversion between relative flux and mmag, as well as time units (i.e. days <-> hours).

@kecnry kecnry removed the waiting-on-jdaviz-release PR requires a release from jdaviz before updating the pin label Jun 7, 2024
@kecnry kecnry marked this pull request as ready for review June 7, 2024 15:20
@kecnry kecnry force-pushed the jdaviz-3.10 branch 4 times, most recently from b325d1f to 47cb252 Compare June 9, 2024 11:30
@kecnry kecnry merged commit 95bb4d8 into spacetelescope:main Jun 11, 2024
10 of 13 checks passed
@kecnry kecnry deleted the jdaviz-3.10 branch June 11, 2024 10:31
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