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

Fix compatibility with glue 1.19+ #110

Closed
wants to merge 6 commits into from
Closed

Conversation

kecnry
Copy link
Member

@kecnry kecnry commented Apr 18, 2024

Reverts #109

Not sure (yet) if this needs upstream fixes (glue and/or jdaviz) or extra handling here to ensure to_object goes through lightkurve instead of specutils.

Waiting for: jdaviz 3.9.1

@kecnry kecnry added the waiting-on-jdaviz-release PR requires a release from jdaviz before updating the pin label Apr 19, 2024
@kecnry kecnry added this to the 3.9.1 milestone Apr 19, 2024
@kecnry kecnry force-pushed the revert-109-max-pin-glue branch from 6e850f9 to df6f3ec Compare April 19, 2024 19:10
@kecnry kecnry modified the milestone: 0.3.1 Apr 19, 2024
@kecnry kecnry force-pushed the revert-109-max-pin-glue branch from df6f3ec to 2611fe6 Compare April 19, 2024 19:41
@kecnry kecnry force-pushed the revert-109-max-pin-glue branch from 2611fe6 to fe6cb7f Compare April 19, 2024 19:44
@kecnry kecnry changed the title Revert "temporarily avoid conflicts introduced by recent glue releases" Fix compatibility with glue 1.19+ Apr 19, 2024
@kecnry kecnry removed the waiting-on-jdaviz-release PR requires a release from jdaviz before updating the pin label Apr 19, 2024
@kecnry kecnry force-pushed the revert-109-max-pin-glue branch from 0369553 to 4397fd9 Compare April 19, 2024 20:10
@kecnry kecnry force-pushed the revert-109-max-pin-glue branch 4 times, most recently from 9e9f9ef to 942382d Compare April 19, 2024 20:29
@kecnry kecnry force-pushed the revert-109-max-pin-glue branch from 942382d to 692970d Compare April 19, 2024 20:36
lcviz/helper.py Outdated
Comment on lines 23 to 25
# for some reason, glue is trying to request a change for cid='flux' from d to electron / s
if target_units not in self.equivalent_units(data, cid, original_units):
return values
Copy link
Member Author

Choose a reason for hiding this comment

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

not sure why this would be necessary. If you comment this out and put a print statement in its place, you can see that glue is calling to_unit to convert between time and flux units. @dhomeier - any ideas? Is this safe for now or not a good idea?

Choose a reason for hiding this comment

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

I am not surprised it wants some flux-like units for data.get_component('flux').units, the question perhaps still remains, how did the cid get assigned to 'flux' here?

Copy link
Member Author

@kecnry kecnry Apr 19, 2024

Choose a reason for hiding this comment

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

I added a commit that raises the traceback if that helps. From the values, it seems that this is actually the time (dt) component, but getting a request to translate units with and incorrect cid and target_units

>       helper.load_data(tpf)

../../.tox/py310-test-alldeps-cov/lib/python3.10/site-packages/lcviz/tests/test_tray_viewer_creator.py:21: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
../../.tox/py310-test-alldeps-cov/lib/python3.10/site-packages/lcviz/helper.py:137: in load_data
    super().load_data(
../../.tox/py310-test-alldeps-cov/lib/python3.10/site-packages/jdaviz/core/helpers.py:110: in load_data
    self.app.load_data(data, parser_reference=parser_reference, **kwargs)
../../.tox/py310-test-alldeps-cov/lib/python3.10/site-packages/jdaviz/app.py:783: in load_data
    parser(self, file_obj, **kwargs)
../../.tox/py310-test-alldeps-cov/lib/python3.10/site-packages/lcviz/parsers.py:76: in light_curve_parser
    app.add_data_to_viewer('image', new_data_label)
../../.tox/py310-test-alldeps-cov/lib/python3.10/site-packages/jdaviz/app.py:1502: in add_data_to_viewer
    self.set_data_visibility(viewer_item, data_label, visible=visible, replace=clear_other_data)
../../.tox/py310-test-alldeps-cov/lib/python3.10/site-packages/jdaviz/app.py:2036: in set_data_visibility
    viewer.add_data(data, percentile=95, color=viewer.color_cycler())
../../.tox/py310-test-alldeps-cov/lib/python3.10/site-packages/glue_jupyter/view.py:118: in add_data
    result = super().add_data(data)
../../.tox/py310-test-alldeps-cov/lib/python3.10/site-packages/glue/viewers/common/viewer.py:209: in add_data
    layer = get_layer_artist_from_registry(data, self) or self.get_data_layer_artist(data)
../../.tox/py310-test-alldeps-cov/lib/python3.10/site-packages/glue_jupyter/bqplot/image/viewer.py:101: in get_data_layer_artist
    return self.get_layer_artist(cls, layer=layer, layer_state=layer_state)
../../.tox/py310-test-alldeps-cov/lib/python3.10/site-packages/glue_jupyter/view.py:143: in get_layer_artist
    return cls(self, self.state, layer=layer, layer_state=layer_state)
../../.tox/py310-test-alldeps-cov/lib/python3.10/site-packages/glue_jupyter/bqplot/image/layer_artist.py:21: in __init__
    super().__init__(view, *args, **kwargs)
../../.tox/py310-test-alldeps-cov/lib/python3.10/site-packages/glue/viewers/image/layer_artist.py:68: in __init__
    super(ImageLayerArtist, self).__init__(axes, viewer_state,
../../.tox/py310-test-alldeps-cov/lib/python3.10/site-packages/glue/viewers/image/layer_artist.py:31: in __init__
    super(BaseImageLayerArtist, self).__init__(axes, viewer_state,
../../.tox/py310-test-alldeps-cov/lib/python3.10/site-packages/glue/viewers/matplotlib/layer_artist.py:18: in __init__
    super(MatplotlibLayerArtist, self).__init__(viewer_state,
../../.tox/py310-test-alldeps-cov/lib/python3.10/site-packages/glue/viewers/common/layer_artist.py:29: in __init__
    self._viewer_state.layers.append(self.state)
../../.tox/py310-test-alldeps-cov/lib/python3.10/site-packages/echo/containers.py:52: in append
    self.notify_all()
../../.tox/py310-test-alldeps-cov/lib/python3.10/site-packages/echo/containers.py:45: in notify_all
    callback(*args, **kwargs)
../../.tox/py310-test-alldeps-cov/lib/python3.10/site-packages/echo/containers.py:166: in __call__
    self.function(*args, **kwargs)
../../.tox/py310-test-alldeps-cov/lib/python3.10/site-packages/echo/containers.py:189: in callback
    self.notify(instance, wrapped_list, wrapped_list)
../../.tox/py310-test-alldeps-cov/lib/python3.10/site-packages/echo/core.py:125: in notify
    cback(new)
../../.tox/py310-test-alldeps-cov/lib/python3.10/site-packages/lcviz/viewers.py:353: in _on_layers_update
    layer.attribute = flux_comp
../../.tox/py310-test-alldeps-cov/lib/python3.10/site-packages/echo/core.py:261: in __setattr__
    super(HasCallbackProperties, self).__setattr__(attribute, value)
../../.tox/py310-test-alldeps-cov/lib/python3.10/site-packages/echo/selection.py:41: in __set__
    super(SelectionCallbackProperty, self).__set__(instance, value)
../../.tox/py310-test-alldeps-cov/lib/python3.10/site-packages/echo/core.py:76: in __set__
    self.notify(instance, old, new)
../../.tox/py310-test-alldeps-cov/lib/python3.10/site-packages/glue/utils/matplotlib.py:170: in wrapper
    return func(*args, **kwargs)
../../.tox/py310-test-alldeps-cov/lib/python3.10/site-packages/glue/viewers/matplotlib/state.py:38: in notify
    super(DeferredDrawSelectionCallbackProperty, self).notify(*args, **kwargs)
../../.tox/py310-test-alldeps-cov/lib/python3.10/site-packages/echo/core.py:125: in notify
    cback(new)
../../.tox/py310-test-alldeps-cov/lib/python3.10/site-packages/glue/viewers/image/state.py:607: in _update_attribute_display_unit_choices
    self.attribute_display_unit = component.units
../../.tox/py310-test-alldeps-cov/lib/python3.10/site-packages/echo/core.py:263: in __setattr__
    self._notify_global(**{attribute: value})
../../.tox/py310-test-alldeps-cov/lib/python3.10/site-packages/glue/utils/matplotlib.py:170: in wrapper
    return func(*args, **kwargs)
../../.tox/py310-test-alldeps-cov/lib/python3.10/site-packages/glue/viewers/matplotlib/state.py:319: in _notify_global
    super(MatplotlibLayerState, self)._notify_global(*args, **kwargs)
../../.tox/py310-test-alldeps-cov/lib/python3.10/site-packages/echo/core.py:258: in _notify_global
    callback(**kwargs)
../../.tox/py310-test-alldeps-cov/lib/python3.10/site-packages/glue/core/state_objects.py:206: in _update_values
    self.update_values(**properties)
../../.tox/py310-test-alldeps-cov/lib/python3.10/site-packages/glue/core/state_objects.py:346: in update_values
    limits_native = converter.to_native(self.data,
../../.tox/py310-test-alldeps-cov/lib/python3.10/site-packages/glue/core/units.py:40: in to_native
    return self.converter_helper.to_unit(data, cid, values, original_units, target_units)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <lcviz.helper.UnitConverter object at 0x7f9adcc7a6e0>
data = Data (label: contents Q1), cid = flux
values = array([ 0.        , 33.[471](https://github.com/spacetelescope/lcviz/actions/runs/8760564540/job/24045734274?pr=110#step:5:472)63724]), original_units = 'd'
target_units = 'electron / s'

    def to_unit(self, data, cid, values, original_units, target_units):
        # for some reason, glue is trying to request a change for cid='flux' from d to electron / s
        if target_units not in self.equivalent_units(data, cid, original_units):
>           raise ValueError(f"to_unit cannot convert units, cid={cid}, original_units={original_units}, target_units={target_units}, values={values}")  # noqa
E           ValueError: to_unit cannot convert units, cid=flux, original_units=d, target_units=electron / s, values=[ 0.         33.471[637](https://github.com/spacetelescope/lcviz/actions/runs/8760611587/job/24045862391?pr=110#step:5:638)24]

Copy link
Member Author

@kecnry kecnry Apr 23, 2024

Choose a reason for hiding this comment

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

This traceback can be reproduced with this branch of lcviz (with the raise ValueError included in the if-statment), jdaviz released (3.9.1) or main, and glue 1.19+.

from lightkurve import search_targetpixelfile
from lcviz import LCviz

tpf = search_targetpixelfile("HAT-P-11", mission="Kepler", cadence="long", quarter=10).download()

lcviz = LCviz()
lcviz.load_data(tpf)

Choose a reason for hiding this comment

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

Just for the record, got the traceback I just mentioned with released glue-core==1.18.0, jdaviz==3.9.1, lcviz==0.3.0 on lcviz.load_data(tpf):

UnboundLocalError                         Traceback (most recent call last)
Cell In[3], line 2
      1 lcviz = LCviz()
----> 2 lcviz.load_data(tpf)

File ~/opt/miniforge/envs/lcviz/lib/python3.12/site-packages/lcviz/helper.py:118, in LCviz.load_data(self, data, data_label)
    100 def load_data(self, data, data_label=None):
    101     """
    102     Load data into LCviz.
    103 
   (...)
    116         appended for clarity.
    117     """
--> 118     super().load_data(
    119         data=data,
    120         parser_reference='light_curve_parser',
    121         data_label=data_label
    122     )

File ~/opt/miniforge/envs/lcviz/lib/python3.12/site-packages/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 ~/opt/miniforge/envs/lcviz/lib/python3.12/site-packages/jdaviz/app.py:783, in Application.load_data(self, file_obj, parser_reference, **kwargs)
    780         parser = data_parser_registry.members.get(data_parser)
    782 if parser is not None:
--> 783     parser(self, file_obj, **kwargs)
    784 else:
    785     self._application_handler.load_data(file_obj)

File ~/opt/miniforge/envs/lcviz/lib/python3.12/site-packages/lcviz/parsers.py:29, in light_curve_parser(app, file_obj, data_label, show_in_viewer, **kwargs)
     27     new_data_label = f'{data_label}'
     28 else:
---> 29     new_data_label = light_curve.meta.get('OBJECT', 'Light curve')
     31 # handle flux_origin default
     32 flux_origin = light_curve.meta.get('FLUX_ORIGIN', None)  # i.e. PDCSAP or SAP

UnboundLocalError: cannot access local variable 'light_curve' where it is not associated with a value

With the current main branch this works up to displaying the TPF, but I don't see much functionality in selecting another frame or time.

Copy link

@dhomeier dhomeier Apr 23, 2024

Choose a reason for hiding this comment

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

Screenshot is with main (same with this branch) and glue 1.18; there are no exceptions raised, just trying to figure out how this is supposed to work (e.g. what the time axis is mapped to).

Choose a reason for hiding this comment

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

With this branch and 1.20 I am also getting a units-related error, but at a different point than #110 (comment) in setting up a callback:

----> 2 lcviz.load_data(tpf)

File ~/opt/miniforge/envs/lcviz/lib/python3.12/site-packages/lcviz/helper.py:137, in LCviz.load_data(self, data, data_label)
    119 def load_data(self, data, data_label=None):
    120     """
    121     Load data into LCviz.
    122 
   (...)
    135         appended for clarity.
    136     """
--> 137     super().load_data(
    138         data=data,
    139         parser_reference='light_curve_parser',
    140         data_label=data_label
    141     )

File ~/opt/miniforge/envs/lcviz/lib/python3.12/site-packages/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 ~/opt/miniforge/envs/lcviz/lib/python3.12/site-packages/jdaviz/app.py:783, in Application.load_data(self, file_obj, parser_reference, **kwargs)
    780         parser = data_parser_registry.members.get(data_parser)
    782 if parser is not None:
--> 783     parser(self, file_obj, **kwargs)
    784 else:
    785     self._application_handler.load_data(file_obj)

File ~/opt/miniforge/envs/lcviz/lib/python3.12/site-packages/lcviz/parsers.py:76, in light_curve_parser(app, file_obj, data_label, show_in_viewer, **kwargs)
     73         if not found_viewer:
     74             app._on_new_viewer(NewViewerMessage(CubeView, data=None, sender=app),
     75                                vid='image', name='image')
---> 76             app.add_data_to_viewer('image', new_data_label)
     78 else:
     79     if show_in_viewer:

File ~/opt/miniforge/envs/lcviz/lib/python3.12/site-packages/jdaviz/app.py:1502, in Application.add_data_to_viewer(self, viewer_reference, data_label, visible, clear_other_data)
   1499 if viewer_item is None:
   1500     raise ValueError(f"Could not identify viewer with reference {viewer_reference}")
-> 1502 self.set_data_visibility(viewer_item, data_label, visible=visible, replace=clear_other_data)

File ~/opt/miniforge/envs/lcviz/lib/python3.12/site-packages/jdaviz/app.py:2036, in Application.set_data_visibility(self, viewer_reference, data_label, visible, replace)
   2030     raise ValueError(
   2031         f"No data item found with label '{data_label}'. Label must be one "
   2032         "of:\n\t" + "\n\t".join(dc_labels))
   2034 data = self.data_collection[data_label]
-> 2036 viewer.add_data(data, percentile=95, color=viewer.color_cycler())
   2038 # Specviz removes the data from collection in viewer.py if flux unit incompatible.
   2039 if data_label not in self.data_collection:

File ~/opt/miniforge/envs/lcviz/lib/python3.12/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 ~/opt/miniforge/envs/lcviz/lib/python3.12/site-packages/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 ~/opt/miniforge/envs/lcviz/lib/python3.12/site-packages/glue_jupyter/bqplot/image/viewer.py:101, in BqplotImageView.get_data_layer_artist(self, layer, layer_state)
     99 else:
    100     cls = BqplotImageLayerArtist
--> 101 return self.get_layer_artist(cls, layer=layer, layer_state=layer_state)

File ~/opt/miniforge/envs/lcviz/lib/python3.12/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 ~/opt/miniforge/envs/lcviz/lib/python3.12/site-packages/glue_jupyter/bqplot/image/layer_artist.py:21, in BqplotImageLayerArtist.__init__(self, view, *args, **kwargs)
     20 def __init__(self, view, *args, **kwargs):
---> 21     super().__init__(view, *args, **kwargs)
     22     # TODO: we should probably use a lru cache to avoid having a 'memleak'
     23     self._contour_line_cache = {}

File ~/opt/miniforge/envs/lcviz/lib/python3.12/site-packages/glue/viewers/image/layer_artist.py:68, in ImageLayerArtist.__init__(self, axes, viewer_state, layer_state, layer)
     66 def __init__(self, axes, viewer_state, layer_state=None, layer=None):
---> 68     super(ImageLayerArtist, self).__init__(axes, viewer_state,
     69                                            layer_state=layer_state, layer=layer)
     71     # We use a custom object to deal with the compositing of images, and we
     72     # store it as a private attribute of the axes to make sure it is
     73     # accessible for all layer artists.
     74     self.uuid = str(uuid.uuid4())

File ~/opt/miniforge/envs/lcviz/lib/python3.12/site-packages/glue/viewers/image/layer_artist.py:31, in BaseImageLayerArtist.__init__(self, axes, viewer_state, layer_state, layer)
     29 def __init__(self, axes, viewer_state, layer_state=None, layer=None):
---> 31     super(BaseImageLayerArtist, self).__init__(axes, viewer_state,
     32                                                layer_state=layer_state, layer=layer)
     34     # Watch for changes in the viewer state which would require the
     35     # layers to be redrawn
     36     self._viewer_state.add_global_callback(self._update_image)

File ~/opt/miniforge/envs/lcviz/lib/python3.12/site-packages/glue/viewers/matplotlib/layer_artist.py:18, in MatplotlibLayerArtist.__init__(self, axes, viewer_state, layer_state, layer)
     16 def __init__(self, axes, viewer_state, layer_state=None, layer=None):
---> 18     super(MatplotlibLayerArtist, self).__init__(viewer_state,
     19                                                 layer_state=layer_state,
     20                                                 layer=layer)
     21     self.axes = axes
     22     self.mpl_artists = []

File ~/opt/miniforge/envs/lcviz/lib/python3.12/site-packages/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 ~/opt/miniforge/envs/lcviz/lib/python3.12/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 ~/opt/miniforge/envs/lcviz/lib/python3.12/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 ~/opt/miniforge/envs/lcviz/lib/python3.12/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 ~/opt/miniforge/envs/lcviz/lib/python3.12/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 ~/opt/miniforge/envs/lcviz/lib/python3.12/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 ~/opt/miniforge/envs/lcviz/lib/python3.12/site-packages/lcviz/viewers.py:353, in CubeView._on_layers_update(self, layers)
    351 for layer in self.state.layers:
    352     if hasattr(layer, 'attribute') and layer.attribute != flux_comp:
--> 353         layer.attribute = flux_comp

File ~/opt/miniforge/envs/lcviz/lib/python3.12/site-packages/echo/core.py:261, in HasCallbackProperties.__setattr__(self, attribute, value)
    260 def __setattr__(self, attribute, value):
--> 261     super(HasCallbackProperties, self).__setattr__(attribute, value)
    262     if self.is_callback_property(attribute):
    263         self._notify_global(**{attribute: value})

File ~/opt/miniforge/envs/lcviz/lib/python3.12/site-packages/echo/selection.py:41, in SelectionCallbackProperty.__set__(self, instance, value)
     39         if not any(value is x for x in choices):
     40             raise ValueError('value {0} is not in valid choices: {1}'.format(value, choices))
---> 41 super(SelectionCallbackProperty, self).__set__(instance, value)

File ~/opt/miniforge/envs/lcviz/lib/python3.12/site-packages/echo/core.py:76, in CallbackProperty.__set__(self, instance, value)
     74 new = self.__get__(instance)
     75 if old != new:
---> 76     self.notify(instance, old, new)

File ~/opt/miniforge/envs/lcviz/lib/python3.12/site-packages/glue/utils/matplotlib.py:180, in defer_draw.<locals>.wrapper(*args, **kwargs)
    178     for backend in DEFER_DRAW_BACKENDS:
    179         backend.draw_idle = DeferredMethod(backend.draw_idle)
--> 180     result = func(*args, **kwargs)
    181 finally:
    182     for backend in DEFER_DRAW_BACKENDS:
    183         # We need to use another try...finally block here in case the
    184         # executed deferred draw calls fail for any reason

File ~/opt/miniforge/envs/lcviz/lib/python3.12/site-packages/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 ~/opt/miniforge/envs/lcviz/lib/python3.12/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 ~/opt/miniforge/envs/lcviz/lib/python3.12/site-packages/glue/viewers/image/state.py:607, in ImageLayerState._update_attribute_display_unit_choices(self, *args)
    605     c_choices = ['']
    606 ImageLayerState.attribute_display_unit.set_choices(self, c_choices)
--> 607 self.attribute_display_unit = component.units

File ~/opt/miniforge/envs/lcviz/lib/python3.12/site-packages/echo/core.py:263, in HasCallbackProperties.__setattr__(self, attribute, value)
    261 super(HasCallbackProperties, self).__setattr__(attribute, value)
    262 if self.is_callback_property(attribute):
--> 263     self._notify_global(**{attribute: value})

File ~/opt/miniforge/envs/lcviz/lib/python3.12/site-packages/glue/utils/matplotlib.py:175, in defer_draw.<locals>.wrapper(*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):
--> 175     return func(*args, **kwargs)
    177 try:
    178     for backend in DEFER_DRAW_BACKENDS:

File ~/opt/miniforge/envs/lcviz/lib/python3.12/site-packages/glue/viewers/matplotlib/state.py:319, in MatplotlibLayerState._notify_global(self, *args, **kwargs)
    317 @defer_draw
    318 def _notify_global(self, *args, **kwargs):
--> 319     super(MatplotlibLayerState, self)._notify_global(*args, **kwargs)

File ~/opt/miniforge/envs/lcviz/lib/python3.12/site-packages/echo/core.py:258, in HasCallbackProperties._notify_global(self, **kwargs)
    256 if len(kwargs) > 0:
    257     for callback in self._global_callbacks:
--> 258         callback(**kwargs)

File ~/opt/miniforge/envs/lcviz/lib/python3.12/site-packages/glue/core/state_objects.py:206, in StateAttributeCacheHelper._update_values(self, **properties)
    203 properties = dict((self._attribute_lookup_inv[key], value)
    204                   for key, value in properties.items() if key in self._attribute_lookup_inv and self._attribute_lookup_inv[key] != 'attribute')
    205 if len(properties) > 0:
--> 206     self.update_values(**properties)

File ~/opt/miniforge/envs/lcviz/lib/python3.12/site-packages/glue/core/state_objects.py:346, in StateAttributeLimitsHelper.update_values(self, force, use_default_modifiers, **properties)
    344     limits_native = current_limits
    345 else:
--> 346     limits_native = converter.to_native(self.data,
    347                                         self.component_id,
    348                                         current_limits,
    349                                         previous_units)
    351 lower, upper = converter.to_unit(self.data,
    352                                  self.component_id,
    353                                  limits_native,
    354                                  display_units)
    356 self.set(lower=lower, upper=upper)

File ~/opt/miniforge/envs/lcviz/lib/python3.12/site-packages/glue/core/units.py:40, in UnitConverter.to_native(self, data, cid, values, original_units)
     38 target_units = self._get_units(data, cid)
     39 if target_units:
---> 40     return self.converter_helper.to_unit(data, cid, values, original_units, target_units)
     41 else:
     42     return values

File ~/opt/miniforge/envs/lcviz/lib/python3.12/site-packages/lcviz/helper.py:25, in UnitConverter.to_unit(self, data, cid, values, original_units, target_units)
     22 def to_unit(self, data, cid, values, original_units, target_units):
     23     # for some reason, glue is trying to request a change for cid='flux' from d to electron / s
     24     if target_units not in self.equivalent_units(data, cid, original_units):
---> 25         raise ValueError(f"to_unit cannot convert units, cid={cid}, original_units={original_units}, target_units={target_units}, values={values}")  # noqa
     26         return values
     27     return (values * u.Unit(original_units)).to_value(u.Unit(target_units))

ValueError: to_unit cannot convert units, cid=flux, original_units=d, target_units=electron / s, values=[ 0.         93.42335314]

Copy link
Member Author

Choose a reason for hiding this comment

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

if you want to actually use the slicing, then you can add a light curve as well:

from lightkurve import search_lightcurve, search_targetpixelfile
from lcviz import LCviz

lc = search_lightcurve("HAT-P-11", mission="Kepler", cadence="long", quarter=10).download()
tpf = search_targetpixelfile("HAT-P-11", mission="Kepler", cadence="long", quarter=10).download()

lcviz = LCviz()
lcviz.load_data(lc)
lcviz.load_data(tpf)
lcviz.show()

## the default stretch is pretty bad (we're working on this), this helps somewhat:
po = lcviz.plugins['Plot Options']
po.viewer = 'image'
po.image_colormap = 'Gray'
po.stretch_function = 'Arcsinh'
po.stretch_vmin = 0
po.stretch_vmax = 20000

Choose a reason for hiding this comment

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

Thanks, that works better – with the default stretch the image was indeed saturated almost everywhere and thus never seemed to change; also when shown on its own the TPF x range is set to [0, 1] d, when not much is happening at all.

Copy link
Member Author

Choose a reason for hiding this comment

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

@dhomeier - are there any updates on this? I'm migrating this work over to #105 now, but would like to avoid this hacky workaround if possible.

@kecnry kecnry force-pushed the revert-109-max-pin-glue branch from fa3c517 to e8dedb3 Compare April 19, 2024 23:10
@kecnry kecnry mentioned this pull request May 28, 2024
2 tasks
@kecnry kecnry closed this May 29, 2024
@kecnry kecnry deleted the revert-109-max-pin-glue branch May 29, 2024 19:02
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