model fitting: handle empty poly order input

CHANGES.rst

@@ -26,6 +26,7 @@ Bug Fixes
 - Model plugin properly updates parameters after fit for compound models. [#1023]
 - Model plugin now respects fixed parameters when applying model to cube, and retains
   parameter units in that case. [#1026]
+- Model plugin polynomial order now avoids traceback when clearing input. [#1041]
 
 Cubeviz
 ^^^^^^^

docs/dev/ui_style_guide.rst

@@ -25,6 +25,18 @@ try to adhere to the following principles:
 * Use ``<v-row justify="end">`` to align content to the right (such as action buttons).
 * Use new ``<j-plugin-section-header>Header Text</j-plugin-section-header>`` to separate content 
   within a plugin (instead of nested cards, ``v-card-subtitle``, etc).
+* Number entries should use a ``<v-text-field type="number" v-model="traitlet_name">`` component 
+  *unless* requiring support for scientific notation (in which case 
+  ``<v-text-field @change="python_method">`` can be used with stripping invalid characters and
+  type-casting in python).  To handle emptying the input component without raising a traceback,
+  use an ``IntHandleEmpty`` traitlet instead, along with form-validation (see below) and/or
+  checks on the python-side to handle the case of an empty string.
+* Use form validation wherever possible, and disable action buttons if the relevant validation
+  does not pass.  This is preferred to raising errors through snackbars after pressing an action
+  button.  To do this, wrap the relevant section in a ``<v-form v-model="form_valid_section_name">``,
+  create a ``form_valid_section_name = Bool(False).tag(sync=True)`` in the python class for the 
+  plugin, add rules to any relevant inputs, and set ``:disabled="!form_valid_section_name"`` to any
+  action buttons.
 
 .. code::
 
@@ -37,5 +49,29 @@ try to adhere to the following principles:
         <v-row>
           ....
         </v-row>
+
+        <v-form v-model="form_valid">
+          <v-row>
+            <v-text-field
+              label="Label"
+              type="number"
+              v-model.number="int_handle_empty_traitlet"
+              :rules="[() => int_handle_empty_traitlet!=='' || 'This field is required']"
+              hint="Hint text."
+              persistent-hint
+            >
+            </v-text-field>
+          </v-row>
+
+          <v-row jutify="end">
+            <v-btn 
+              color="primary" 
+              text 
+              :disabled="!form_valid"
+              @click="(e) => {add_model(e); validate()}"
+              >Action Text
+            </v-btn>
+          </v-row>
+        </v-form>
       </j-tray-plugin>
     </template>

jdaviz/configs/default/plugins/model_fitting/model_fitting.py

@@ -8,14 +8,15 @@
                                SubsetUpdateMessage)
 from specutils import Spectrum1D, SpectralRegion
 from specutils.utils import QuantityModel
-from traitlets import Any, Bool, Int, List, Unicode, observe
+from traitlets import Any, Bool, List, Unicode, observe
 from glue.core.data import Data
 from glue.core.subset import Subset, RangeSubsetState, OrState, AndState
 from glue.core.link_helpers import LinkSame
 
 from jdaviz.core.events import AddDataMessage, RemoveDataMessage, SnackbarMessage
 from jdaviz.core.registries import tray_registry
 from jdaviz.core.template_mixin import TemplateMixin
+from jdaviz.core.custom_traitlets import IntHandleEmpty
 from jdaviz.configs.default.plugins.model_fitting.fitting_backend import fit_model_to_spectrum
 from jdaviz.configs.default.plugins.model_fitting.initializers import (MODELS,
                                                                        initialize,
@@ -37,7 +38,10 @@ class ModelFitting(TemplateMixin):
     dialog = Bool(False).tag(sync=True)
     template_file = __file__, "model_fitting.vue"
     dc_items = List([]).tag(sync=True)
+    form_valid_data_selection = Bool(False).tag(sync=True)
+    form_valid_model_component = Bool(False).tag(sync=True)
 
+    selected_data = Unicode("").tag(sync=True)
     spectral_min = Any().tag(sync=True)
     spectral_max = Any().tag(sync=True)
     spectral_unit = Unicode().tag(sync=True)
@@ -52,7 +56,7 @@ class ModelFitting(TemplateMixin):
     eq_error = Bool(False).tag(sync=True)
     component_models = List([]).tag(sync=True)
     display_order = Bool(False).tag(sync=True)
-    poly_order = Int(0).tag(sync=True)
+    poly_order = IntHandleEmpty(0).tag(sync=True)
 
     # add/replace results for "fit"
     add_replace_results = Bool(True).tag(sync=True)
@@ -79,7 +83,6 @@ def __init__(self, *args, **kwargs):
         self._initialized_models = {}
         self._display_order = False
         self.model_label = "Model"
-        self._selected_data_label = None
         self._spectral_subsets = {}
         self._window = None
         self._original_mask = None
@@ -245,7 +248,8 @@ def _warn_if_no_equation(self):
         else:
             return False
 
-    def vue_data_selected(self, event):
+    @observe("selected_data")
+    def _selected_data_changed(self, event):
         """
         Callback method for when the user has selected data from the drop down
         in the front-end. It is here that we actually parse and create a new
@@ -260,16 +264,14 @@ def vue_data_selected(self, event):
             label of the data collection object selected by the user.
         """
         selected_spec = self.app.get_data_from_viewer("spectrum-viewer",
-                                                      data_label=event)
+                                                      data_label=event['new'])
         # Replace NaNs from collapsed SpectralCube in Cubeviz
         # (won't affect calculations because these locations are masked)
         selected_spec.flux[np.isnan(selected_spec.flux)] = 0.0
 
         # Save original mask so we can reset after applying a subset mask
         self._original_mask = selected_spec.mask
 
-        self._selected_data_label = event
-
         if self._units == {}:
             self._units["x"] = str(
                 selected_spec.spectral_axis.unit)
@@ -471,16 +473,16 @@ def vue_fit_model_to_cube(self, *args, **kwargs):
         if self._warn_if_no_equation():
             return
 
-        if self._selected_data_label in self.app.data_collection.labels:
-            data = self.app.data_collection[self._selected_data_label]
+        if self.selected_data in self.app.data_collection.labels:
+            data = self.app.data_collection[self.selected_data]
         else:  # User selected some subset from spectrum viewer, just use original cube
             data = self.app.data_collection[0]
 
         # First, ensure that the selected data is cube-like. It is possible
         # that the user has selected a pre-existing 1d data object.
         if data.ndim != 3:
             snackbar_message = SnackbarMessage(
-                f"Selected data {self._selected_data_label} is not cube-like",
+                f"Selected data {self.selected_data} is not cube-like",
                 color='error',
                 sender=self)
             self.hub.broadcast(snackbar_message)
@@ -586,7 +588,7 @@ def vue_register_spectrum(self, event):
         self.app.add_data(spectrum, label)
 
         # Make sure we link the result spectrum to the data we're fitting
-        data_fitted = self.app.session.data_collection[self._selected_data_label]
+        data_fitted = self.app.session.data_collection[self.selected_data]
         data_id = data_fitted.world_component_ids[0]
         model_id = self.app.session.data_collection[label].world_component_ids[0]
         self.app.session.data_collection.add_link(LinkSame(data_id, model_id))

jdaviz/configs/default/plugins/model_fitting/model_fitting.vue

@@ -4,67 +4,80 @@
       <j-docs-link :link="'https://jdaviz.readthedocs.io/en/'+vdocs+'/'+config+'/plugins.html#model-fitting'">Fit an analytic model to data or a subset.</j-docs-link>
     </v-row>
 
-    <v-row>
-      <v-select
-        :items="dc_items"
-        @change="data_selected"
-        label="Data"
-        hint="Select the data set to be fitted."
-        persistent-hint
-      ></v-select>
-    </v-row>
+    <v-form v-model="form_valid_data_selection">
+      <v-row>
+        <v-select
+          :items="dc_items"
+          v-model="selected_data"
+          label="Data"
+          hint="Select the data set to be fitted."
+          :rules="[() => !!selected_data || 'This field is required']"
+          persistent-hint
+        ></v-select>
+      </v-row>
 
-    <v-row>
-      <v-select
-        :items="spectral_subset_items"
-        v-model="selected_subset"
-        label="Spectral Region"
-        hint="Select spectral region to fit."
-        persistent-hint
-        @click="list_subsets"
-      ></v-select>
-    </v-row>
+      <v-row>
+        <v-select
+          :items="spectral_subset_items"
+          v-model="selected_subset"
+          label="Spectral Region"
+          hint="Select spectral region to fit."
+          persistent-hint
+          @click="list_subsets"
+        ></v-select>
+      </v-row>
+    </v-form>
 
     <j-plugin-section-header>Model Components</j-plugin-section-header>
-    <v-row>
-      <v-select
-        :items="available_models"
-        @change="model_selected"
-        label="Model"
-        hint="Select a model to fit."
-        persistent-hint
-      ></v-select>
-    </v-row>
+    <v-form v-model="form_valid_model_component">
+      <v-row>
+        <v-select
+          :items="available_models"
+          @change="model_selected"
+          label="Model"
+          hint="Select a model to fit."
+          persistent-hint
+          :rules="[() => model_selected!=='' || 'This field is required']"
+        ></v-select>
+      </v-row>
 
-    <v-row>
-      <v-text-field
-        label="Model ID"
-        v-model="temp_name"
-        @change="sanitizeModelId"
-        hint="A unique string label for this component model."
-        persistent-hint
-        :rules="[() => !!temp_name || 'This field is required',
-                 () => component_models.map((item) => item.id).indexOf(temp_name) === -1 || 'ID already in use']"
-      >
-      </v-text-field>
-    </v-row>
+      <v-row>
+        <v-text-field
+          label="Model ID"
+          v-model="temp_name"
+          @change="sanitizeModelId"
+          hint="A unique string label for this component model."
+          persistent-hint
+          :rules="[() => temp_name!=='' || 'This field is required',
+                   () => component_models.map((item) => item.id).indexOf(temp_name) === -1 || 'ID already in use']"
+        >
+        </v-text-field>
+      </v-row>
 
-    <v-row v-if="display_order">
-      <v-text-field
-        label="Order"
-        type="number"
-        v-model.number="poly_order"
-        hint="Order of polynomial to fit."
-        persistent-hint
-      >
-      </v-text-field>
-    </v-row>
+      <v-row v-if="display_order">
+        <v-text-field
+          label="Order"
+          type="number"
+          v-model.number="poly_order"
+          :rules="[() => poly_order!=='' || 'This field is required']"
+          hint="Order of polynomial to fit."
+          persistent-hint
+        >
+        </v-text-field>
+      </v-row>
 
-    <v-row justify="end">
-      <j-tooltip tipid='plugin-model-fitting-add-model'>
-        <v-btn color="primary" text @click="add_model">Add Component</v-btn>
-      </j-tooltip>
-    </v-row>
+      <v-row justify="end">
+        <j-tooltip tipid='plugin-model-fitting-add-model'>
+          <v-btn 
+            color="primary" 
+            text 
+            :disabled="!form_valid_model_component || !form_valid_data_selection"
+            @click="add_model"
+            >Add Component
+          </v-btn>
+        </j-tooltip>
+      </v-row>
+    </v-form>
 
     <div v-if="component_models.length">
       <j-plugin-section-header>Model Parameters</j-plugin-section-header>

jdaviz/core/custom_traitlets.py

@@ -0,0 +1,26 @@
+from traitlets import Int, Float
+
+
+class HandleEmptyMixin:
+    def __init__(self, *args, **kwargs):
+        self._empty_to_default = kwargs.pop('replace_with_default', False)
+        super().__init__(*args, **kwargs)
+
+    def validate(self, obj, value):
+        if isinstance(value, str) and not len(value):
+            if self._empty_to_default:
+                # If the field is emptied, it will override with the default value.
+                return self.default_value
+            # The value will remain as the empty string, likely will need to either
+            # couple this with form validation or handle the case where the value
+            # is an empty string once retrieved.
+            return value
+        return super().validate(obj, value)
+
+
+class IntHandleEmpty(HandleEmptyMixin, Int):
+    pass
+
+
+class FloatHandleEmpty(HandleEmptyMixin, Float):
+    pass

jdaviz/core/tests/test_custom_traitlets.py

@@ -0,0 +1,28 @@
+import pytest
+from traitlets import HasTraits, TraitError
+
+from jdaviz.core.custom_traitlets import IntHandleEmpty, FloatHandleEmpty
+
+
+class Foo(HasTraits):
+    int_handle_empty = IntHandleEmpty(0)
+    int_handle_empty_replace = IntHandleEmpty(0, replace_with_default=True)
+    float_handle_empty = FloatHandleEmpty()
+
+
+def test_inthandleempty():
+    foo = Foo()
+
+    foo.int_handle_empty = 1
+    foo.int_handle_empty = ''
+    assert foo.int_handle_empty == ''
+    with pytest.raises(
+            TraitError,
+            match=r"The 'int_handle_empty' trait of a Foo instance expected an int, not the str 'blah'\."):  # noqa
+        foo.int_handle_empty = 'blah'
+
+    foo.int_handle_empty_replace = 1
+    foo.int_handle_empty_replace = ''
+    assert foo.int_handle_empty_replace == 0
+
+    foo.float_handle_empty = 1.2