Skip to content

Commit

Permalink
user-APIs: forbid setting non-existing attribute (#2577)
Browse files Browse the repository at this point in the history
* user-APIs: forbid setting non-existing attribute
* fix broken tests
  • Loading branch information
kecnry authored Nov 28, 2023
1 parent db7c9f1 commit ffb585b
Show file tree
Hide file tree
Showing 6 changed files with 15 additions and 7 deletions.
3 changes: 3 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ API Changes

- Deprecated ``app.get_subsets_from_viewer`` is removed, use ``viz_helper.get_subsets`` instead. [#2578]

- User APIs now raise a warning when attempting to set a non-existing attribute to avoid confusion
caused by typos, etc. [#2577]

Cubeviz
^^^^^^^

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ def test_markers_layer_cycle(self):
label_mouseover = self.imviz.app.session.application._tools['g-coords-info']

mp = self.imviz.plugins['Markers']
mp.plugin_opened = True
mp._obj.plugin_opened = True

# cycle through dataset options (used for both coords info and markers)
assert label_mouseover.dataset.choices == ['auto', 'none',
Expand Down Expand Up @@ -217,7 +217,7 @@ def test_markers_custom_viewer(self):
label_mouseover = self.imviz.app.session.application._tools['g-coords-info']

mp = self.imviz.plugins['Markers']
mp.plugin_opened = True
mp._obj.plugin_opened = True

nv = self.imviz.create_image_viewer()
self.imviz.app.add_data_to_viewer('imviz-1', 'has_wcs[SCI,1]')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def test_custom_model_labels(specviz_helper, spectrum1d):

for i, model in enumerate(MODELS):
# Add one of each model with a unique name
modelfit_plugin.model_comp_selected = model
modelfit_plugin._obj.model_comp_selected = model
modelfit_plugin._obj.comp_label = f"test_model_{i}"
modelfit_plugin._obj.vue_add_model({})

Expand Down Expand Up @@ -159,7 +159,7 @@ def test_refit_plot_options(specviz_helper, spectrum1d):
specviz_helper.load_data(spectrum1d)
modelfit_plugin = specviz_helper.plugins['Model Fitting']

modelfit_plugin.model_comp_selected = 'Const1D'
modelfit_plugin._obj.model_comp_selected = 'Const1D'
modelfit_plugin._obj.comp_label = "C"
modelfit_plugin._obj.vue_add_model({})

Expand Down Expand Up @@ -190,6 +190,9 @@ def test_user_api(specviz_helper, spectrum1d):
specviz_helper.load_data(spectrum1d)
p = specviz_helper.plugins['Model Fitting']

with pytest.raises(ValueError, match="blah is not a valid attribute and cannot be set"):
p.blah = 5

# even though the default label is set to C, adding Linear1D should default to its automatic
# default label of 'L'
assert p.model_component == 'Const1D' # tests SelectPluginComponent's __eq__
Expand Down
2 changes: 1 addition & 1 deletion jdaviz/configs/imviz/tests/test_catalogs.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ def test_plugin_image_with_result(self, imviz_helper, tmp_path):
self.imviz = imviz_helper

catalogs_plugin = self.imviz.app.get_tray_item_from_name('imviz-catalogs')
catalogs_plugin.plugin_opened = True
catalogs_plugin._obj.plugin_opened = True
# This basically calls the following under the hood:
# skycoord_center = SkyCoord(6.62754354, 1.54466139, unit="deg")
# zoom_radius = r_max = 3 * u.arcmin
Expand Down
2 changes: 1 addition & 1 deletion jdaviz/configs/imviz/tests/test_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ def test_blink(imviz_helper):

def test_compass_open_while_load(imviz_helper):
plg = imviz_helper.plugins['Compass']
plg.plugin_opened = True
plg._obj.plugin_opened = True

# Should not crash even if Compass is open in tray.
imviz_helper.load_data(np.ones((2, 2)))
Expand Down
4 changes: 3 additions & 1 deletion jdaviz/core/user_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,10 @@ def __getattr__(self, attr):
return getattr(exp_obj, 'user_api', exp_obj)

def __setattr__(self, attr, value):
if attr in _internal_attrs or attr not in self._expose:
if attr in _internal_attrs:
return super().__setattr__(attr, value)
if attr not in self._expose:
raise ValueError(f"{attr} is not a valid attribute and cannot be set")

if attr in self._readonly:
raise AttributeError("cannot set read-only item")
Expand Down

0 comments on commit ffb585b

Please sign in to comment.