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

increase test coverage #39

Merged
merged 3 commits into from
Aug 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 1 addition & 1 deletion lcviz/helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def _get_range_subset_bounds(self, subset_state, *args, **kwargs):
# TODO: use display units once implemented in Glue for ScatterViewer
# units = u.Unit(viewer.state.x_display_unit)
units = u.Unit(viewer.time_unit)
else:
else: # pragma: no cover
Copy link
Contributor

Choose a reason for hiding this comment

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

Well, that's one way to improve coverage 😉

Copy link
Member Author

Choose a reason for hiding this comment

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

it's my favorite way! (I just figured we didn't need coverage on simple input-errors, so might as well have that officially noted)

raise ValueError("Unable to find time axis units")

region = reference_time + u.Quantity([subset_state.lo * units, subset_state.hi * units])
Expand Down
18 changes: 3 additions & 15 deletions lcviz/parsers.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
import os
from astropy.io import fits
from glue.config import data_translator
from jdaviz.core.registries import data_parser_registry
from lightkurve import LightCurve, KeplerLightCurve, TessLightCurve
from lightkurve.io.detect import detect_filetype
import lightkurve

__all__ = ["light_curve_parser"]

Expand All @@ -17,21 +15,11 @@ def light_curve_parser(app, file_obj, data_label=None, show_in_viewer=True, **kw
if data_label is None:
data_label = os.path.splitext(os.path.basename(file_obj))[0]

# detect the type light curve in a FITS file:
with fits.open(file_obj) as hdulist:
filetype = detect_filetype(hdulist)

# get the constructor for this type of light curve:
filetype_to_cls = {
'KeplerLightCurve': KeplerLightCurve,
'TessLightCurve': TessLightCurve
}
cls = filetype_to_cls[filetype]
# read the light curve:
light_curve = cls.read(file_obj)
light_curve = lightkurve.read(file_obj)
Comment on lines -20 to +19
Copy link
Member Author

Choose a reason for hiding this comment

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

@bmorris3 - any reason why we can't rely on lightkurve functionality here?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm pretty sure this was a workaround in #10 until lightkurve/lightkurve#1299 was merged (see #10 (comment)). If this is simplification is working correctly now, then we're all good!


# load a LightCurve object:
elif isinstance(file_obj, LightCurve):
elif isinstance(file_obj, lightkurve.LightCurve):
light_curve = file_obj

# make a data label:
Expand Down
6 changes: 3 additions & 3 deletions lcviz/plugins/ephemeris/ephemeris.py
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ def _on_component_remove(self, lbl):
_ = self._ephemerides.pop(lbl, {})
# remove the corresponding viewer, if it exists
viewer_item = self.app._viewer_item_by_id(self._phase_viewer_id(lbl))
if viewer_item is None:
if viewer_item is None: # pragma: no cover
return
cid = viewer_item.get('id', None)
if cid is not None:
Expand Down Expand Up @@ -346,7 +346,7 @@ def update_ephemeris(self, component=None, t0=None, period=None, dpdt=None):
if component is None:
component = self.component_selected

if component not in self.component.choices:
if component not in self.component.choices: # pragma: no cover
raise ValueError(f"component must be one of {self.component.choices}")

existing_ephem = self._ephemerides.get(component, {})
Expand Down Expand Up @@ -411,7 +411,7 @@ def _update_periodogram(self, *args):
self.method_spinner = False
self.method_err = str(err)
return
else:
else: # pragma: no cover
self.method_spinner = False
raise NotImplementedError(f"periodogram not implemented for {self.method}")

Expand Down
4 changes: 2 additions & 2 deletions lcviz/plugins/flatten/flatten.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ def marks(self):
@observe('default_to_overwrite', 'dataset_selected')
def _set_default_results_label(self, event={}):
'''Generate a label and set the results field to that value'''
if not hasattr(self, 'dataset'):
if not hasattr(self, 'dataset'): # pragma: no cover
return

self.add_results.label_whitelist_overwrite = [self.dataset_selected]
Expand Down Expand Up @@ -125,7 +125,7 @@ def flatten(self, add_data=True):
The trend used to flatten the light curve.
"""
input_lc = self.dataset.selected_obj
if input_lc is None:
if input_lc is None: # pragma: no cover
raise ValueError("no input dataset selected")

output_lc, trend_lc = input_lc.flatten(return_trend=True,
Expand Down
8 changes: 4 additions & 4 deletions lcviz/state.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ def _reset_att_limits(self, ax):
# override glue's _reset_x/y_limits to account for all layers,
# not just reference data
att = f'{ax}_att'
if getattr(self, att) is None:
if getattr(self, att) is None: # pragma: no cover
return

ax_min, ax_max = np.inf, -np.inf
Expand All @@ -21,14 +21,14 @@ def _reset_att_limits(self, ax):
ax_min = min(ax_min, np.nanmin(ax_data))
ax_max = max(ax_max, np.nanmax(ax_data))

if not np.all(np.isfinite([ax_min, ax_max])):
if not np.all(np.isfinite([ax_min, ax_max])): # pragma: no cover
return

with delay_callback(self, f'{ax}_min', f'{ax}_max'):
setattr(self, f'{ax}_min', ax_min)
setattr(self, f'{ax}_max', ax_max)

def _reset_x_limit(self, *event):
def _reset_x_limits(self, *event):
self._reset_att_limits('x')

def _reset_y_limits(self, *event):
Expand All @@ -39,7 +39,7 @@ def reset_limits(self, *event):
y_min, y_max = np.inf, -np.inf

for layer in self.layers:
if not layer.visible:
if not layer.visible: # pragma: no cover
continue

x_data = layer.layer.data.get_data(self.x_att)
Expand Down
83 changes: 42 additions & 41 deletions lcviz/tests/test_parser.py
Original file line number Diff line number Diff line change
@@ -1,52 +1,53 @@
# import pytest
import pytest
import numpy as np
from glue.core.roi import XRangeROI, YRangeROI
from astropy.time import Time
# from astropy.utils.data import download_file
from astropy.utils.data import download_file
from lightkurve import LightCurve
# from lightkurve.io import kepler
from lightkurve.io import kepler
import astropy.units as u

from lcviz.utils import TimeCoordinates

# @pytest.mark.remote_data
# def test_kepler_via_mast_local_file(helper):
# url = (
# 'https://archive.stsci.edu/pub/kepler/'
# 'lightcurves/0014/001429092/kplr001429092-2009166043257_llc.fits'
# ) # 188 KB
#
# path = download_file(url, cache=True, timeout=100)
# helper.load_data(path)
#
# data = helper.app.data_collection[0]
# flux_arr = data['flux']
# flux_unit = u.Unit(data.get_component('flux').units)
# flux = flux_arr * flux_unit
#
# assert isinstance(data.coords, TimeCoordinates)
# assert isinstance(flux, u.Quantity)
# assert flux.unit.is_equivalent(u.electron / u.s)
#
#
# @pytest.mark.remote_data
# def test_kepler_via_mast_preparsed(helper):
# url = (
# 'https://archive.stsci.edu/pub/kepler/'
# 'lightcurves/0014/001429092/kplr001429092-2009166043257_llc.fits'
# ) # 188 KB
#
# light_curve = kepler.read_kepler_lightcurve(url)
# helper.load_data(light_curve)
#
# data = helper.app.data_collection[0]
# flux_arr = data['flux']
# flux_unit = u.Unit(data.get_component('flux').units)
# flux = flux_arr * flux_unit
#
# assert isinstance(data.coords, TimeCoordinates)
# assert isinstance(flux, u.Quantity)
# assert flux.unit.is_equivalent(u.electron / u.s)

@pytest.mark.remote_data
def test_kepler_via_mast_local_file(helper):
url = (
'https://archive.stsci.edu/pub/kepler/'
'lightcurves/0014/001429092/kplr001429092-2009166043257_llc.fits'
) # 188 KB

path = download_file(url, cache=True, timeout=100)
helper.load_data(path)

data = helper.app.data_collection[0]
flux_arr = data['flux']
flux_unit = u.Unit(data.get_component('flux').units)
flux = flux_arr * flux_unit

assert isinstance(data.coords, TimeCoordinates)
assert isinstance(flux, u.Quantity)
assert flux.unit.is_equivalent(u.electron / u.s)


@pytest.mark.remote_data
def test_kepler_via_mast_preparsed(helper):
url = (
'https://archive.stsci.edu/pub/kepler/'
'lightcurves/0014/001429092/kplr001429092-2009166043257_llc.fits'
) # 188 KB

light_curve = kepler.read_kepler_lightcurve(url)
helper.load_data(light_curve)

data = helper.app.data_collection[0]
flux_arr = data['flux']
flux_unit = u.Unit(data.get_component('flux').units)
flux = flux_arr * flux_unit

assert isinstance(data.coords, TimeCoordinates)
assert isinstance(flux, u.Quantity)
assert flux.unit.is_equivalent(u.electron / u.s)


def test_synthetic_lc(helper):
Expand Down
16 changes: 16 additions & 0 deletions lcviz/tests/test_plugin_ephemeris.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,22 @@ def test_plugin_ephemeris(helper, light_curve_like_kepler_quarter):
assert 'renamed custom component' in ephem.ephemerides
assert len(helper.app.get_viewer_ids()) == 3

assert ephem.component == 'renamed custom component'
assert ephem.period == 3.14
assert ephem.ephemeris['period'] == 3.14
# modify the ephemeris of the NON-selected ephemeris component
ephem.update_ephemeris(component='default', period=2)
assert ephem.period == 3.14
assert ephem.ephemerides['default']['period'] == 2

ephem.remove_component('renamed custom component')
assert len(ephem.ephemerides) == 1
assert len(helper.app.get_viewer_ids()) == 2
assert ephem.component == 'default'
assert ephem.period == 2

assert ephem.method.selected == 'Lomb-Scargle'
ephem.method = 'Box Least Squares'
assert ephem._obj.method_err == ''
ephem._obj.vue_adopt_period_at_max_power()
assert ephem.period != 2
53 changes: 41 additions & 12 deletions lcviz/tests/test_plugin_flatten.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,27 +4,56 @@
from lcviz.marks import LivePreviewTrend, LivePreviewFlattened


def _get_marks_from_viewer(viewer, cls=(LivePreviewTrend, LivePreviewFlattened)):
return [m for m in viewer.figure.marks if isinstance(m, cls)]
def _get_marks_from_viewer(viewer, cls=(LivePreviewTrend, LivePreviewFlattened),
include_not_visible=False):
return [m for m in viewer.figure.marks if isinstance(m, cls)
if include_not_visible or m.visible]


def test_plugin_flatten(helper, light_curve_like_kepler_quarter):
helper.load_data(light_curve_like_kepler_quarter)
tv = helper.app.get_viewer(helper._default_time_viewer_reference_name)

f = helper.plugins['Flatten']
f.plugin_opened = True
ephem = helper.plugins['Ephemeris']
pv = ephem.create_phase_viewer()
f = helper.plugins['Flatten']

assert len(_get_marks_from_viewer(tv)) == 2
assert len(_get_marks_from_viewer(pv)) == 1

orig_label = f.dataset.selected
assert f.dataset.selected_obj is not None
assert f._obj.add_results.label_overwrite is True
assert f._obj.add_results.label == orig_label
f.flatten(add_data=True)
# no marks until plugin opened/active
assert len(_get_marks_from_viewer(tv)) == 0
assert len(_get_marks_from_viewer(pv)) == 0

with f.as_active():
assert len(_get_marks_from_viewer(tv)) == 2
assert len(_get_marks_from_viewer(pv)) == 1

# update period which should update phasing in phase-viewer
ephem.period = 1.2

before_polyorder = f.polyorder
before_update = _get_marks_from_viewer(tv)[0].y

# test error handling in live-preview
f.polyorder = -1
assert f._obj.flatten_err != ''
assert len(_get_marks_from_viewer(tv)) == 0
assert len(_get_marks_from_viewer(pv)) == 0

# update polyorder (live-preview should re-appear and have changed from before)
f.polyorder = before_polyorder + 1
assert f._obj.flatten_err == ''
after_update = _get_marks_from_viewer(tv)[0].y
assert not np.allclose(before_update, after_update)

orig_label = f.dataset.selected
assert f.dataset.selected_obj is not None
assert f._obj.add_results.label_overwrite is True
assert f._obj.add_results.label == orig_label
f._obj.vue_apply(add_data=True)
assert f._obj.flatten_err == ''

# marks are hidden
assert len(_get_marks_from_viewer(tv)) == 0
assert len(_get_marks_from_viewer(pv)) == 0


def test_no_overwrite(helper, light_curve_like_kepler_quarter):
Expand Down
19 changes: 19 additions & 0 deletions lcviz/tests/test_viewers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@

def test_reset_limits(helper, light_curve_like_kepler_quarter):
helper.load_data(light_curve_like_kepler_quarter)
tv = helper.app.get_viewer(helper._default_time_viewer_reference_name)

orig_xlims = (tv.state.x_min, tv.state.x_max)
orig_ylims = (tv.state.y_min, tv.state.y_max)
# set xmin and ymin to midpoints
new_xmin = (tv.state.x_min + tv.state.x_max) / 2
new_ymin = (tv.state.y_min + tv.state.y_max) / 2
tv.state.x_min = new_xmin
tv.state.y_min = new_ymin

tv.state._reset_x_limits()
assert tv.state.x_min == orig_xlims[0]
assert tv.state.y_min == new_ymin

tv.state._reset_y_limits()
assert tv.state.y_min == orig_ylims[0]
6 changes: 3 additions & 3 deletions lcviz/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class TimeCoordinates(Coordinates):
"""

def __init__(self, times, reference_time=None, unit=u.d):
if not isinstance(times, Time):
if not isinstance(times, Time): # pragma: no cover
raise TypeError('values should be a Time instance')
self._index = np.arange(len(times))
self._times = times
Expand All @@ -44,15 +44,15 @@ def time_axis(self):
return self._times

def world_to_pixel_values(self, *world):
if len(world) > 1:
if len(world) > 1: # pragma: no cover
raise ValueError('TimeCoordinates is a 1-d coordinate class '
'and only accepts a single scalar or array to convert')
return interp1d(
self._values.value, self._index, fill_value='extrapolate'
)(world[0])

def pixel_to_world_values(self, *pixel):
if len(pixel) > 1:
if len(pixel) > 1: # pragma: no cover
raise ValueError('SpectralCoordinates is a 1-d coordinate class '
'and only accepts a single scalar or array to convert')
return interp1d(
Expand Down