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
18 changes: 9 additions & 9 deletions .github/workflows/ci_workflows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,24 +51,24 @@ jobs:
toxposargs: --remote-data
allow_failure: false

- name: OS X - Python 3.9
- name: OS X - Python 3.10
os: macos-latest
python: 3.9
toxenv: py39-test
python: '3.10'
toxenv: py310-test
allow_failure: false

- name: Windows - Python 3.9
- name: Windows - Python 3.11
os: windows-latest
python: 3.9
toxenv: py39-test
python: 3.11
toxenv: py311-test
allow_failure: false

# This also runs on cron but we want to make sure new changes
# won't break this job at the PR stage.
- name: Python 3.11 with latest dev versions of key dependencies, and remote data
- name: Python 3.12 with latest dev versions of key dependencies, and remote data
os: ubuntu-latest
python: '3.11'
toxenv: py311-test-devdeps
python: '3.12'
toxenv: py312-test-devdeps
toxposargs: --remote-data --run-slow
allow_failure: true

Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/publish.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ jobs:

- uses: actions/setup-python@v5
with:
python-version: 3.9
python-version: '3.10'

- name: Install python-build and twine
run: python -m pip install build "twine>=3.3"
Expand Down
4 changes: 4 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
0.4.0 (unreleased)
------------------

* Updates to use jdaviz 3.10, which now requires python 3.10+. [#105]

* Support loading, viewing, and slicing through TPF data cubes. [#82, #117]

* Support loading, viewing, and slicing through TPF data cubes. [#82, #117, #118]

* Default data labels no longer include flux-origin, but do include quarter/campaign/sector. [#111]
Expand Down
2 changes: 1 addition & 1 deletion docs/installation.rst
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ Common Issues
If you encounter problems while following these installation instructions,
please consult :ref:`known installation issues <known_issues_installation>`.

Note that ``lcviz`` requires Python 3.9 or newer. If your ``pip`` corresponds to an older version of
Note that ``lcviz`` requires Python 3.10 or newer. If your ``pip`` corresponds to an older version of
Python, it will raise an error that it cannot find a valid package.

Users occasionally encounter problems running the pure ``pip`` install above. For those
Expand Down
38 changes: 38 additions & 0 deletions lcviz/helper.py
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:
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).

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
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 Expand Up @@ -57,6 +74,23 @@
return


def _get_display_unit(app, axis):
if app._jdaviz_helper is None or app._jdaviz_helper.plugins.get('Unit Conversion') is None: # noqa
# fallback on native units (unit conversion is not enabled)
if axis == 'time':
return u.d
elif axis == 'flux':
return app._jdaviz_helper.default_time_viewer._obj.data()[0].flux.unit

Check warning on line 83 in lcviz/helper.py

View check run for this annotation

Codecov / codecov/patch

lcviz/helper.py#L82-L83

Added lines #L82 - L83 were not covered by tests
else:
raise ValueError(f"could not find units for axis='{axis}'")
try:

Check warning on line 86 in lcviz/helper.py

View check run for this annotation

Codecov / codecov/patch

lcviz/helper.py#L85-L86

Added lines #L85 - L86 were not covered by tests
# TODO: need to implement and add unit conversion plugin for this to be able to work
return getattr(app._jdaviz_helper.plugins.get('Unit Conversion')._obj,

Check warning on line 88 in lcviz/helper.py

View check run for this annotation

Codecov / codecov/patch

lcviz/helper.py#L88

Added line #L88 was not covered by tests
f'{axis}_unit_selected')
except AttributeError:
raise ValueError(f"could not find display unit for axis='{axis}'")

Check warning on line 91 in lcviz/helper.py

View check run for this annotation

Codecov / codecov/patch

lcviz/helper.py#L90-L91

Added lines #L90 - L91 were not covered by tests


class LCviz(ConfigHelper):
_default_configuration = {
'settings': {'configuration': 'lcviz',
Expand Down Expand Up @@ -92,6 +126,10 @@
lambda *args, **kwargs: _link_new_data(self.app, *args, **kwargs)
)

self.app._get_display_unit = (
lambda *args, **kwargs: _get_display_unit(self.app, *args, **kwargs)
)

# inject custom css from lcviz_style.vue (on top of jdaviz styles)
self.app._add_style((__file__, 'lcviz_style.vue'))

Expand Down
6 changes: 5 additions & 1 deletion lcviz/plugins/coords_info/coords_info.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,11 @@
self.icon = 'mdi-cursor-default'
self._dict['data_label'] = ''
else:
time = viewer.slice_value
try:
time = viewer.slice_value
except IndexError:
self._viewer_mouse_clear_event(viewer)
return

Check warning on line 201 in lcviz/plugins/coords_info/coords_info.py

View check run for this annotation

Codecov / codecov/patch

lcviz/plugins/coords_info/coords_info.py#L199-L201

Added lines #L199 - L201 were not covered by tests
# TODO: store slice unit within image viewer to avoid this assumption?
time_unit = str(self.app._jdaviz_helper.default_time_viewer._obj.time_unit)
self.row2_title = 'Time'
Expand Down
2 changes: 1 addition & 1 deletion lcviz/plugins/time_selector/time_selector.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def __init__(self, *args, **kwargs):
handler=self._on_ephemeris_changed)

@property
def slice_axis(self):
def slice_display_unit_name(self):
# global display unit "axis" corresponding to the slice axis
return 'time'

Expand Down
9 changes: 5 additions & 4 deletions lcviz/tests/test_plugin_markers.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,17 +146,18 @@ def test_tpf_markers(helper, light_curve_like_kepler_quarter):
{'event': 'mousemove',
'domain': {'x': 0, 'y': 0}})

assert label_mouseover.as_text() == ('Pixel x=00000.0 y=00000.0 Value +1.28035e+01 electron / s', # noqa
'Time 47.00689 d',
assert label_mouseover.as_text() == ('Pixel x=00000.0 y=00000.0 Value +1.27220e+00 electron / s', # noqa
'Time 47.00000 d',
'')

print(label_mouseover.as_dict())
_assert_dict_allclose(label_mouseover.as_dict(), {'data_label': 'KIC 1429092[TPF]',
'time': 47.00688790508866,
'time': 47.00000,
'time:unit': 'd',
'pixel': (0.0, 0.0),
'axes_x': 0,
'axes_x:unit': 'pix',
'axes_y': 0,
'axes_y:unit': 'pix',
'value': 12.803528785705566,
'value': 1.272199273109436,
'value:unit': 'electron / s'})
8 changes: 8 additions & 0 deletions lcviz/viewers.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,10 @@ def slice_component_label(self):
# calling data_collection_item.get_component(slice_component_label) must work
return 'dt'

@property
def slice_display_unit_name(self):
return 'time'

def data(self, cls=None):
data = []

Expand Down Expand Up @@ -334,6 +338,10 @@ def slice_index(self):
# index in viewer.slices corresponding to the slice axis
return 0

@property
def slice_display_unit_name(self):
return 'time'

def _initial_x_axis(self, *args):
# Make sure that the x_att/y_att is correct on data load
# called via a callback set upstream in CubevizImageView when reference_data is changed
Expand Down
5 changes: 2 additions & 3 deletions pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[project]
name = "lcviz"
description = "A Jdaviz-based light curve analysis and visualization tool"
requires-python = ">=3.9"
requires-python = ">=3.10"
authors = [
{ name = "JDADF Developers", email = "[email protected]" },
]
Expand All @@ -20,10 +20,9 @@ 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.*",
"jdaviz>=3.10.2,<3.11",
"lightkurve>=2.4.1",
]
dynamic = [
Expand Down
Loading