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.

Sign up for GitHub

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

Fix compatibility with glue 1.19+ #110

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e8d0d90
Revert "temporarily avoid conflicts introduced by recent glue release…
kecnry Apr 18, 2024
a6d7b09
update jdaviz pin to pull in upstream fix
kecnry Apr 19, 2024
fe6cb7f
changelog entry
kecnry Apr 19, 2024
7c82d59
bypass jdaviz unit converter
kecnry Apr 19, 2024
692970d
avoid error for incompat unit conversion request
kecnry Apr 19, 2024
e8dedb3
TMP: expose traceback for incompat unit conversion
kecnry Apr 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGES.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@

* Raise error when parser can't identify file_obj [#106]

* Fixes compatibility with glue >= 1.19. [#109, #110]

0.3.0 (04-05-2024)
--------------------

Expand Down
17 changes: 17 additions & 0 deletions lcviz/helper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,30 @@

from lightkurve import LightCurve

from glue.config import settings as glue_settings
from glue.core.component_id import ComponentID
from glue.core.link_helpers import LinkSame
from glue.core.units import unit_converter
from jdaviz.core.helpers import ConfigHelper
from lcviz.viewers import TimeScatterView

__all__ = ['LCviz']


@unit_converter('custom-lcviz')
class UnitConverter:
def equivalent_units(self, data, cid, units):
return set(list(map(str, u.Unit(units).find_equivalent_units(include_prefix_units=True))))

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):
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?

Copy link

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 •
edited
Loading

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 •
edited
Loading

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)

Copy link

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 •
edited
Loading

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).

Copy link

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

Copy link

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.

return (values * u.Unit(original_units)).to_value(u.Unit(target_units))


glue_settings.UNIT_CONVERTER = 'custom-lcviz'

custom_components = {'plugin-ephemeris-select': 'components/plugin_ephemeris_select.vue'}

# Register pure vue component. This allows us to do recursive component instantiation only in the
Expand Down
1 change: 0 additions & 1 deletion pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ classifiers = [
]
dependencies = [
"astropy>=5.2",
"glue-core<1.19.0",
# NOTE: if/when we stop pinning a minor version of jdaviz, add jdaviz
# to devdeps in tox.ini
"jdaviz==3.9.*",
Expand Down
Loading