Merge pull request #1784 from alicevision/fix/uidNodes

Fix and prevent mismatches between an attribute's type and its default value's type

13 files changed, 140 insertions, 35 deletions

diff --git a/meshroom/core/__init__.py b/meshroom/core/__init__.py
index 56d6351d..425806c6 100644
--- a/meshroom/core/__init__.py
+++ b/meshroom/core/__init__.py
@@ -80,10 +80,20 @@ def loadPlugins(folder, packageName, classType):
                            and issubclass(plugin, classType)]
                 if not plugins:
                     logging.warning("No class defined in plugin: {}".format(pluginModuleName))
+
+                importPlugin = True
                 for p in plugins:
+                    if classType == desc.Node:
+                        nodeErrors = validateNodeDesc(p)
+                        if nodeErrors:
+                            errors.append("  * {}: The following parameters do not have valid default values/ranges: {}"
+                                          .format(pluginName, ", ".join(nodeErrors)))
+                            importPlugin = False
+                            break
                     p.packageName = packageName
                     p.packageVersion = packageVersion
-                pluginTypes.extend(plugins)
+                if importPlugin:
+                    pluginTypes.extend(plugins)
             except Exception as e:
                 errors.append('  * {}: {}'.format(pluginName, str(e)))
 
@@ -94,6 +104,38 @@ def loadPlugins(folder, packageName, classType):
     return pluginTypes
 
 
+def validateNodeDesc(nodeDesc):
+    """
+    Check that the node has a valid description before being loaded. For the description
+    to be valid, the default value of every parameter needs to correspond to the type
+    of the parameter.
+    An empty returned list means that every parameter is valid, and so is the node's description.
+    If it is not valid, the returned list contains the names of the invalid parameters. In case
+    of nested parameters (parameters in groups or lists, for example), the name of the parameter
+    follows the name of the parent attributes. For example, if the attribute "x", contained in group
+    "group", is invalid, then it will be added to the list as "group:x".
+
+    Args:
+        nodeDesc (desc.Node): description of the node
+
+    Returns:
+        errors (list): the list of invalid parameters if there are any, empty list otherwise
+    """
+    errors = []
+
+    for param in nodeDesc.inputs:
+        err = param.checkValueTypes()
+        if err:
+            errors.append(err)
+
+    for param in nodeDesc.outputs:
+        err = param.checkValueTypes()
+        if err:
+            errors.append(err)
+
+    return errors
+
+
 class Version(object):
     """
     Version provides convenient properties and methods to manipulate and compare versions.
diff --git a/meshroom/core/desc.py b/meshroom/core/desc.py
index e66eb053..6766f729 100755..100644
--- a/meshroom/core/desc.py
+++ b/meshroom/core/desc.py
@@ -44,6 +44,15 @@ class Attribute(BaseObject):
         """
         raise NotImplementedError("Attribute.validateValue is an abstract function that should be implemented in the derived class.")
 
+    def checkValueTypes(self):
+        """ Returns the attribute's name if the default value's type is invalid or if the range's type (when available)
+        is invalid, empty string otherwise.
+
+        Returns:
+            string: the attribute's name if the default value's or range's type is invalid, empty string otherwise
+        """
+        raise NotImplementedError("Attribute.checkValueTypes is an abstract function that should be implemented in the derived class.")
+
     def matchDescription(self, value, strict=True):
         """ Returns whether the value perfectly match attribute's description.
 
@@ -85,6 +94,9 @@ class ListAttribute(Attribute):
             raise ValueError('ListAttribute only supports list/tuple input values (param:{}, value:{}, type:{})'.format(self.name, value, type(value)))
         return value
 
+    def checkValueTypes(self):
+        return self.elementDesc.checkValueTypes()
+
     def matchDescription(self, value, strict=True):
         """ Check that 'value' content matches ListAttribute's element description. """
         if not super(ListAttribute, self).matchDescription(value, strict):
@@ -133,6 +145,24 @@ class GroupAttribute(Attribute):
 
         return value
 
+    def checkValueTypes(self):
+        """ Check the default value's and range's (if available) type of every attribute contained in the group
+        (including nested attributes).
+
+        Returns an empty string if all the attributes' types are valid, or concatenates the names of the attributes in
+        the group with invalid types.
+        """
+        invalidParams = []
+        for attr in self.groupDesc:
+            name = attr.checkValueTypes()
+            if name:
+                invalidParams.append(name)
+        if invalidParams:
+            # In group "group", if parameters "x" and "y" (with "y" in nested group "subgroup") are invalid, the
+            # returned string will be: "group:x, group:subgroup:y"
+            return self.name + ":" + str(", " + self.name + ":").join(invalidParams)
+        return ""
+
     def matchDescription(self, value, strict=True):
         """
         Check that 'value' contains the exact same set of keys as GroupAttribute's group description
@@ -185,6 +215,13 @@ class File(Attribute):
             raise ValueError('File only supports string input  (param:{}, value:{}, type:{})'.format(self.name, value, type(value)))
         return os.path.normpath(value).replace('\\', '/') if value else ''
 
+    def checkValueTypes(self):
+        # Some File values are functions generating a string: check whether the value is a string or if it
+        # is a function (but there is no way to check that the function's output is indeed a string)
+        if not isinstance(self.value, pyCompatibility.basestring) and not callable(self.value):
+            return self.name
+        return ""
+
 
 class BoolParam(Param):
     """
@@ -201,6 +238,11 @@ class BoolParam(Param):
         except:
             raise ValueError('BoolParam only supports bool value (param:{}, value:{}, type:{})'.format(self.name, value, type(value)))
 
+    def checkValueTypes(self):
+        if not isinstance(self.value, bool):
+            return self.name
+        return ""
+
 
 class IntParam(Param):
     """
@@ -218,6 +260,11 @@ class IntParam(Param):
         except:
             raise ValueError('IntParam only supports int value (param:{}, value:{}, type:{})'.format(self.name, value, type(value)))
 
+    def checkValueTypes(self):
+        if not isinstance(self.value, int) or (self.range and not all([isinstance(r, int) for r in self.range])):
+            return self.name
+        return ""
+
     range = Property(VariantList, lambda self: self._range, constant=True)
 
 
@@ -234,6 +281,11 @@ class FloatParam(Param):
         except:
             raise ValueError('FloatParam only supports float value (param:{}, value:{}, type:{})'.format(self.name, value, type(value)))
 
+    def checkValueTypes(self):
+        if not isinstance(self.value, float) or (self.range and not all([isinstance(r, float) for r in self.range])):
+            return self.name
+        return ""
+
     range = Property(VariantList, lambda self: self._range, constant=True)
 
 
@@ -263,6 +315,10 @@ class ChoiceParam(Param):
             raise ValueError('Non exclusive ChoiceParam value should be iterable (param:{}, value:{}, type:{})'.format(self.name, value, type(value)))
         return [self.conformValue(v) for v in value]
 
+    def checkValueTypes(self):
+        # nothing to validate
+        return ""
+
     values = Property(VariantList, lambda self: self._values, constant=True)
     exclusive = Property(bool, lambda self: self._exclusive, constant=True)
     joinChar = Property(str, lambda self: self._joinChar, constant=True)
@@ -279,6 +335,11 @@ class StringParam(Param):
             raise ValueError('StringParam value should be a string (param:{}, value:{}, type:{})'.format(self.name, value, type(value)))
         return value
 
+    def checkValueTypes(self):
+        if not isinstance(self.value, pyCompatibility.basestring):
+            return self.name
+        return ""
+
 
 class Level(Enum):
     NONE = 0
diff --git a/meshroom/nodes/aliceVision/CameraInit.py b/meshroom/nodes/aliceVision/CameraInit.py
index 7e7bd38c..9bc0e4e2 100644
--- a/meshroom/nodes/aliceVision/CameraInit.py
+++ b/meshroom/nodes/aliceVision/CameraInit.py
@@ -34,8 +34,8 @@ Intrinsic = [
                     "So this value is used to limit the range of possible values in the optimization. \n"
                     "If you put -1, this value will not be used and the focal length will not be bounded.",
                     value=-1.0, uid=[0], range=None),
-    desc.FloatParam(name="focalLength", label="Focal Length", description="Known/Calibrated Focal Length (in mm)", value=1000, uid=[0], range=(0, 10000, 1)),
-    desc.FloatParam(name="pixelRatio", label="pixel Ratio", description="ratio between pixel width and pixel height", value=1, uid=[0], range=(0, 10, 0.1)),
+    desc.FloatParam(name="focalLength", label="Focal Length", description="Known/Calibrated Focal Length (in mm)", value=1000.0, uid=[0], range=(0.0, 10000.0, 1.0)),
+    desc.FloatParam(name="pixelRatio", label="pixel Ratio", description="ratio between pixel width and pixel height", value=1.0, uid=[0], range=(0.0, 10.0, 0.1)),
     desc.BoolParam(name='pixelRatioLocked', label='Pixel ratio Locked',
                    description='the pixelRatio value is locked for estimation',
                    value=True, uid=[0]),
@@ -53,12 +53,12 @@ Intrinsic = [
                      value="", values=['', 'pinhole', 'radial1', 'radial3', 'brown', 'fisheye4', 'equidistant_r3', '3deanamorphic4', '3declassicld', '3deradial4'], exclusive=True, uid=[0]),
     desc.IntParam(name="width", label="Width", description="Image Width", value=0, uid=[0], range=(0, 10000, 1)),
     desc.IntParam(name="height", label="Height", description="Image Height", value=0, uid=[0], range=(0, 10000, 1)),
-    desc.FloatParam(name="sensorWidth", label="Sensor Width", description="Sensor Width (mm)", value=36, uid=[0], range=(0, 1000, 1)),
-    desc.FloatParam(name="sensorHeight", label="Sensor Height", description="Sensor Height (mm)", value=24, uid=[0], range=(0, 1000, 1)),
+    desc.FloatParam(name="sensorWidth", label="Sensor Width", description="Sensor Width (mm)", value=36.0, uid=[0], range=(0.0, 1000.0, 1.0)),
+    desc.FloatParam(name="sensorHeight", label="Sensor Height", description="Sensor Height (mm)", value=24.0, uid=[0], range=(0.0, 1000.0, 1.0)),
     desc.StringParam(name="serialNumber", label="Serial Number", description="Device Serial Number (Camera UID and Lens UID combined)", value="", uid=[0]),
     desc.GroupAttribute(name="principalPoint", label="Principal Point", description="Position of the Optical Center in the Image (i.e. the sensor surface).", groupDesc=[
-        desc.FloatParam(name="x", label="x", description="", value=0, uid=[0], range=(0, 10000, 1)),
-        desc.FloatParam(name="y", label="y", description="", value=0, uid=[0], range=(0, 10000, 1)),
+        desc.FloatParam(name="x", label="x", description="", value=0.0, uid=[0], range=(0.0, 10000.0, 1.0)),
+        desc.FloatParam(name="y", label="y", description="", value=0.0, uid=[0], range=(0.0, 10000.0, 1.0)),
         ]),
 
     desc.ChoiceParam(name="initializationMode", label="Initialization Mode",
@@ -168,7 +168,7 @@ The metadata needed are:
             label='Default Field Of View',
             description='Empirical value for the field of view in degree.',
             value=45.0,
-            range=(0, 180.0, 1),
+            range=(0.0, 180.0, 1.0),
             uid=[],
             advanced=True,
         ),
diff --git a/meshroom/nodes/aliceVision/DepthMap.py b/meshroom/nodes/aliceVision/DepthMap.py
index dbccfe66..e1d84225 100644
--- a/meshroom/nodes/aliceVision/DepthMap.py
+++ b/meshroom/nodes/aliceVision/DepthMap.py
@@ -59,7 +59,7 @@ Use a downscale factor of one (full-resolution) only if the quality of the input
             label='Max View Angle',
             description='Maximum angle between two views.',
             value=70.0,
-            range=(10.0, 120.0, 1),
+            range=(10.0, 120.0, 1.0),
             uid=[0],
             advanced=True,
         ),
@@ -231,7 +231,7 @@ Use a downscale factor of one (full-resolution) only if the quality of the input
             name='refineSigma',
             label='Refine: Sigma',
             description='Refine: Sigma Threshold.',
-            value=15,
+            value=15.0,
             range=(0.0, 30.0, 0.5),
             uid=[0],
             advanced=True,
diff --git a/meshroom/nodes/aliceVision/DepthMapFilter.py b/meshroom/nodes/aliceVision/DepthMapFilter.py
index f69de340..5043d074 100644
--- a/meshroom/nodes/aliceVision/DepthMapFilter.py
+++ b/meshroom/nodes/aliceVision/DepthMapFilter.py
@@ -45,7 +45,7 @@ This allows to filter unstable points before starting the fusion of all depth ma
             label='Max View Angle',
             description='Maximum angle between two views.',
             value=70.0,
-            range=(10.0, 120.0, 1),
+            range=(10.0, 120.0, 1.0),
             uid=[0],
             advanced=True,
         ),
diff --git a/meshroom/nodes/aliceVision/ImageMasking.py b/meshroom/nodes/aliceVision/ImageMasking.py
index 93a8540d..eefe335f 100644
--- a/meshroom/nodes/aliceVision/ImageMasking.py
+++ b/meshroom/nodes/aliceVision/ImageMasking.py
@@ -43,7 +43,7 @@ class ImageMasking(desc.CommandLineNode):
                 description='Hue value to isolate in [0,1] range. 0 = red, 0.33 = green, 0.66 = blue, 1 = red.',
                 semantic='color/hue',
                 value=0.33,
-                range=(0, 1, 0.01),
+                range=(0.0, 1.0, 0.01),
                 uid=[0]
             ),
             desc.FloatParam(
@@ -51,7 +51,7 @@ class ImageMasking(desc.CommandLineNode):
                 label='Tolerance',
                 description='Tolerance around the hue value to isolate.',
                 value=0.1,
-                range=(0, 1, 0.01),
+                range=(0.0, 1.0, 0.01),
                 uid=[0]
             ),
             desc.FloatParam(
@@ -59,15 +59,15 @@ class ImageMasking(desc.CommandLineNode):
                 label='Min Saturation',
                 description='Hue is meaningless if saturation is low. Do not mask pixels below this threshold.',
                 value=0.3,
-                range=(0, 1, 0.01),
+                range=(0.0, 1.0, 0.01),
                 uid=[0]
             ),
             desc.FloatParam(
                 name='hsvMaxSaturation',
                 label='Max Saturation',
                 description='Do not mask pixels above this threshold. It might be useful to mask white/black pixels.',
-                value=1,
-                range=(0, 1, 0.01),
+                value=1.0,
+                range=(0.0, 1.0, 0.01),
                 uid=[0]
             ),
             desc.FloatParam(
@@ -75,15 +75,15 @@ class ImageMasking(desc.CommandLineNode):
                 label='Min Value',
                 description='Hue is meaningless if value is low. Do not mask pixels below this threshold.',
                 value=0.3,
-                range=(0, 1, 0.01),
+                range=(0.0, 1.0, 0.01),
                 uid=[0]
             ),
             desc.FloatParam(
                 name='hsvMaxValue',
                 label='Max Value',
                 description='Do not mask pixels above this threshold. It might be useful to mask white/black pixels.',
-                value=1,
-                range=(0, 1, 0.01),
+                value=1.0,
+                range=(0.0, 1.0, 0.01),
                 uid=[0]
             ),
         ]),
diff --git a/meshroom/nodes/aliceVision/KeyframeSelection.py b/meshroom/nodes/aliceVision/KeyframeSelection.py
index c855cce3..e1037cd3 100644
--- a/meshroom/nodes/aliceVision/KeyframeSelection.py
+++ b/meshroom/nodes/aliceVision/KeyframeSelection.py
@@ -84,7 +84,7 @@ You can extract frames at regular interval by configuring only the min/maxFrameS
                 label="Frame Offset",
                 description="Frame offset.",
                 value=0,
-                range=(0, 100.0, 1.0),
+                range=(0, 100, 1),
                 uid=[0],
             ),
             name="frameOffsets",
diff --git a/meshroom/nodes/aliceVision/Meshing.py b/meshroom/nodes/aliceVision/Meshing.py
index c1a085ed..a64962c8 100644
--- a/meshroom/nodes/aliceVision/Meshing.py
+++ b/meshroom/nodes/aliceVision/Meshing.py
@@ -94,19 +94,19 @@ A Graph Cut Max-Flow is applied to optimally cut the volume. This cut represents
                             name="x", label="x", description="Euler X Rotation",
                             value=0.0,
                             uid=[0],
-                            range=(-90.0, 90.0, 1)
+                            range=(-90.0, 90.0, 1.0)
                         ),
                         desc.FloatParam(
                             name="y", label="y", description="Euler Y Rotation",
                             value=0.0,
                             uid=[0],
-                            range=(-180.0, 180.0, 1)
+                            range=(-180.0, 180.0, 1.0)
                         ),
                         desc.FloatParam(
                             name="z", label="z", description="Euler Z Rotation",
                             value=0.0,
                             uid=[0],
-                            range=(-180.0, 180.0, 1)
+                            range=(-180.0, 180.0, 1.0)
                         )
                     ],
                     joinChar=","
@@ -163,8 +163,8 @@ A Graph Cut Max-Flow is applied to optimally cut the volume. This cut represents
             name='estimateSpaceMinObservationAngle',
             label='Min Observations Angle For SfM Space Estimation',
             description='Minimum angle between two observations for SfM space estimation.',
-            value=10,
-            range=(0, 120, 1),
+            value=10.0,
+            range=(0.0, 120.0, 1.0),
             uid=[0],
             enabled=lambda node: node.estimateSpaceFromSfM.value,
         ),
diff --git a/meshroom/nodes/aliceVision/PanoramaInit.py b/meshroom/nodes/aliceVision/PanoramaInit.py
index 7b341b8f..947f0922 100644
--- a/meshroom/nodes/aliceVision/PanoramaInit.py
+++ b/meshroom/nodes/aliceVision/PanoramaInit.py
@@ -49,7 +49,7 @@ This node allows to setup the Panorama:
             name='yawCW',
             label='Yaw CW',
             description="Yaw ClockWise or CounterClockWise",
-            value=1,
+            value=True,
             uid=[0],
             enabled=lambda node: ('Horizontal' in node.initializeCameras.value) or (node.initializeCameras.value == "Spherical"),
         ),
diff --git a/meshroom/nodes/aliceVision/SfMTransform.py b/meshroom/nodes/aliceVision/SfMTransform.py
index ae4b5625..0466ec50 100644
--- a/meshroom/nodes/aliceVision/SfMTransform.py
+++ b/meshroom/nodes/aliceVision/SfMTransform.py
@@ -99,19 +99,19 @@ The transformation can be based on:
                             name="x", label="x", description="Euler X Rotation",
                             value=0.0,
                             uid=[0],
-                            range=(-90.0, 90.0, 1)
+                            range=(-90.0, 90.0, 1.0)
                         ),
                         desc.FloatParam(
                             name="y", label="y", description="Euler Y Rotation",
                             value=0.0,
                             uid=[0],
-                            range=(-180.0, 180.0, 1)
+                            range=(-180.0, 180.0, 1.0)
                         ),
                         desc.FloatParam(
                             name="z", label="z", description="Euler Z Rotation",
                             value=0.0,
                             uid=[0],
-                            range=(-180.0, 180.0, 1)
+                            range=(-180.0, 180.0, 1.0)
                         )
                     ],
                     joinChar=","
diff --git a/meshroom/nodes/aliceVision/StructureFromMotion.py b/meshroom/nodes/aliceVision/StructureFromMotion.py
index 03b35ad1..4cacd6a1 100644
--- a/meshroom/nodes/aliceVision/StructureFromMotion.py
+++ b/meshroom/nodes/aliceVision/StructureFromMotion.py
@@ -213,7 +213,7 @@ It iterates like that, adding cameras and triangulating new 2D features into 3D
             label='Min Angle For Triangulation',
             description='Minimum angle for triangulation.',
             value=3.0,
-            range=(0.1, 10, 0.1),
+            range=(0.1, 10.0, 0.1),
             uid=[0],
             advanced=True,
         ),
@@ -222,7 +222,7 @@ It iterates like that, adding cameras and triangulating new 2D features into 3D
             label='Min Angle For Landmark',
             description='Minimum angle for landmark.',
             value=2.0,
-            range=(0.1, 10, 0.1),
+            range=(0.1, 10.0, 0.1),
             uid=[0],
             advanced=True,
         ),
@@ -231,7 +231,7 @@ It iterates like that, adding cameras and triangulating new 2D features into 3D
             label='Max Reprojection Error',
             description='Maximum reprojection error.',
             value=4.0,
-            range=(0.1, 10, 0.1),
+            range=(0.1, 10.0, 0.1),
             uid=[0],
             advanced=True,
         ),
@@ -240,7 +240,7 @@ It iterates like that, adding cameras and triangulating new 2D features into 3D
             label='Min Angle Initial Pair',
             description='Minimum angle for the initial pair.',
             value=5.0,
-            range=(0.1, 10, 0.1),
+            range=(0.1, 10.0, 0.1),
             uid=[0],
             advanced=True,
         ),
@@ -249,7 +249,7 @@ It iterates like that, adding cameras and triangulating new 2D features into 3D
             label='Max Angle Initial Pair',
             description='Maximum angle for the initial pair.',
             value=40.0,
-            range=(0.1, 60, 0.1),
+            range=(0.1, 60.0, 0.1),
             uid=[0],
             advanced=True,
         ),
diff --git a/meshroom/nodes/blender/RenderAnimatedCamera.py b/meshroom/nodes/blender/RenderAnimatedCamera.py
index 803fdac6..8e60572f 100644
--- a/meshroom/nodes/blender/RenderAnimatedCamera.py
+++ b/meshroom/nodes/blender/RenderAnimatedCamera.py
@@ -80,7 +80,7 @@ class RenderAnimatedCamera(desc.CommandLineNode):
                     label='Particle Size',
                     description='''Scale of particles used to show the point cloud''',
                     value=0.1,
-                    range=(0.01, 1, 0.01),
+                    range=(0.01, 1.0, 0.01),
                     uid=[0],
                 ),
                 desc.ChoiceParam(
diff --git a/tests/test_multiviewPipeline.py b/tests/test_multiviewPipeline.py
index 4c072fba..f7750a02 100644
--- a/tests/test_multiviewPipeline.py
+++ b/tests/test_multiviewPipeline.py
@@ -115,3 +115,5 @@ def test_multiviewPipeline():
         assert sorted([n.name for n in loadedGraph.nodes]) == sorted([n.name for n in graph.nodes])
         #  - no compatibility issues
         assert all(isinstance(n, Node) for n in loadedGraph.nodes)
+        #  - same UIDs for every node
+        assert sorted([n._uids.get(0) for n in loadedGraph.nodes]) == sorted([n._uids.get(0) for n in graph.nodes])