From ffb585b8af34809410c6945c385e1ecafbe048cf Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Tue, 28 Nov 2023 10:09:09 -0500 Subject: [PATCH] user-APIs: forbid setting non-existing attribute (#2577) * user-APIs: forbid setting non-existing attribute * fix broken tests --- CHANGES.rst | 3 +++ .../default/plugins/markers/tests/test_markers_plugin.py | 4 ++-- .../default/plugins/model_fitting/tests/test_plugin.py | 7 +++++-- jdaviz/configs/imviz/tests/test_catalogs.py | 2 +- jdaviz/configs/imviz/tests/test_tools.py | 2 +- jdaviz/core/user_api.py | 4 +++- 6 files changed, 15 insertions(+), 7 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 9886b38be4..2cebe1f5ba 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -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 ^^^^^^^ diff --git a/jdaviz/configs/default/plugins/markers/tests/test_markers_plugin.py b/jdaviz/configs/default/plugins/markers/tests/test_markers_plugin.py index 4a329195c8..4021ab7965 100644 --- a/jdaviz/configs/default/plugins/markers/tests/test_markers_plugin.py +++ b/jdaviz/configs/default/plugins/markers/tests/test_markers_plugin.py @@ -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', @@ -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]') diff --git a/jdaviz/configs/default/plugins/model_fitting/tests/test_plugin.py b/jdaviz/configs/default/plugins/model_fitting/tests/test_plugin.py index 7b24c7278f..2bfddac85f 100644 --- a/jdaviz/configs/default/plugins/model_fitting/tests/test_plugin.py +++ b/jdaviz/configs/default/plugins/model_fitting/tests/test_plugin.py @@ -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({}) @@ -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({}) @@ -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__ diff --git a/jdaviz/configs/imviz/tests/test_catalogs.py b/jdaviz/configs/imviz/tests/test_catalogs.py index 7465ea0d0e..7e3e12f438 100644 --- a/jdaviz/configs/imviz/tests/test_catalogs.py +++ b/jdaviz/configs/imviz/tests/test_catalogs.py @@ -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 diff --git a/jdaviz/configs/imviz/tests/test_tools.py b/jdaviz/configs/imviz/tests/test_tools.py index a96150013a..4c7a315261 100644 --- a/jdaviz/configs/imviz/tests/test_tools.py +++ b/jdaviz/configs/imviz/tests/test_tools.py @@ -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))) diff --git a/jdaviz/core/user_api.py b/jdaviz/core/user_api.py index b8cd6341ad..6b6d124e96 100644 --- a/jdaviz/core/user_api.py +++ b/jdaviz/core/user_api.py @@ -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")