-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Make label, description, and value optional on all attribute constructors #3009
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
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,10 +1,30 @@ | ||||||||||||||||||
| import ast | ||||||||||||||||||
| import os | ||||||||||||||||||
| import re | ||||||||||||||||||
| from collections.abc import Iterable | ||||||||||||||||||
| from enum import auto, Enum | ||||||||||||||||||
|
|
||||||||||||||||||
| from meshroom.common import BaseObject, JSValue, Property, Variant, VariantList, strtobool | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
||||||||||||||||||
| def _labelFromName(name): | ||||||||||||||||||
| """Convert a camelCase or snake_case attribute name to a 'Title Case' label. | ||||||||||||||||||
|
|
||||||||||||||||||
| Examples: | ||||||||||||||||||
| >>> _labelFromName('myAttributeName') | ||||||||||||||||||
| 'My Attribute Name' | ||||||||||||||||||
| >>> _labelFromName('my_attribute_name') | ||||||||||||||||||
| 'My Attribute Name' | ||||||||||||||||||
| >>> _labelFromName('input') | ||||||||||||||||||
| 'Input' | ||||||||||||||||||
| """ | ||||||||||||||||||
| # Insert space before uppercase letter following a lowercase letter or digit (camelCase) | ||||||||||||||||||
| s = re.sub(r'([a-z\d])([A-Z])', r'\1 \2', name) | ||||||||||||||||||
| # Replace underscores with spaces (snake_case) | ||||||||||||||||||
| s = s.replace('_', ' ') | ||||||||||||||||||
| # Capitalize each word | ||||||||||||||||||
| return ' '.join(word.capitalize() for word in s.split()) | ||||||||||||||||||
|
|
||||||||||||||||||
| class ValueTypeErrors(Enum): | ||||||||||||||||||
| NONE = auto() # No error | ||||||||||||||||||
| TYPE = auto() # Invalid type | ||||||||||||||||||
|
|
@@ -20,8 +40,8 @@ def __init__(self, name, label, description, value, advanced, semantic, group, e | |||||||||||||||||
| validValue=True, errorMessage="", visible=True, exposed=False): | ||||||||||||||||||
| super(Attribute, self).__init__() | ||||||||||||||||||
| self._name = name | ||||||||||||||||||
| self._label = label | ||||||||||||||||||
| self._description = description | ||||||||||||||||||
| self._label = label if label is not None else _labelFromName(name) | ||||||||||||||||||
| self._description = description if description is not None else "" | ||||||||||||||||||
| self._value = value | ||||||||||||||||||
| self._keyable = keyable | ||||||||||||||||||
| self._keyType = keyType | ||||||||||||||||||
|
|
@@ -134,7 +154,7 @@ def matchDescription(self, value, strict=True): | |||||||||||||||||
|
|
||||||||||||||||||
| class ListAttribute(Attribute): | ||||||||||||||||||
| """ A list of Attributes """ | ||||||||||||||||||
| def __init__(self, elementDesc, name, label, description, group="allParams", advanced=False, semantic="", | ||||||||||||||||||
| def __init__(self, elementDesc, name, label=None, description=None, group="allParams", advanced=False, semantic="", | ||||||||||||||||||
| enabled=True, joinChar=" ", visible=True, exposed=False): | ||||||||||||||||||
| """ | ||||||||||||||||||
| :param elementDesc: the Attribute description of elements to store in that list | ||||||||||||||||||
|
|
@@ -186,7 +206,7 @@ def matchDescription(self, value, strict=True): | |||||||||||||||||
|
|
||||||||||||||||||
| class GroupAttribute(Attribute): | ||||||||||||||||||
| """ A macro Attribute composed of several Attributes """ | ||||||||||||||||||
| def __init__(self, items, name, label, description, group="allParams", advanced=False, semantic="", | ||||||||||||||||||
| def __init__(self, items, name, label=None, description=None, group="allParams", advanced=False, semantic="", | ||||||||||||||||||
| enabled=True, joinChar=" ", brackets=None, visible=True, exposed=False): | ||||||||||||||||||
| """ | ||||||||||||||||||
| :param items: the description of the Attributes composing this group | ||||||||||||||||||
|
|
@@ -294,8 +314,8 @@ def retrieveChildrenInvalidations(self): | |||||||||||||||||
| class Param(Attribute): | ||||||||||||||||||
| """ | ||||||||||||||||||
| """ | ||||||||||||||||||
| def __init__(self, name, label, description, value, group, advanced, semantic, enabled, | ||||||||||||||||||
| keyable=False, keyType=None, invalidate=True, uidIgnoreValue=None, | ||||||||||||||||||
| def __init__(self, name, label=None, description=None, value=None, group="allParams", advanced=False, semantic="", | ||||||||||||||||||
| enabled=True, keyable=False, keyType=None, invalidate=True, uidIgnoreValue=None, | ||||||||||||||||||
| validValue=True, errorMessage="", visible=True, exposed=False): | ||||||||||||||||||
| super(Param, self).__init__(name=name, label=label, description=description, value=value, | ||||||||||||||||||
| keyable=keyable, keyType=keyType, group=group, advanced=advanced, | ||||||||||||||||||
|
|
@@ -307,7 +327,7 @@ def __init__(self, name, label, description, value, group, advanced, semantic, e | |||||||||||||||||
| class File(Attribute): | ||||||||||||||||||
| """ | ||||||||||||||||||
| """ | ||||||||||||||||||
| def __init__(self, name, label, description, value, group="allParams", advanced=False, invalidate=True, | ||||||||||||||||||
| def __init__(self, name, label=None, description=None, value=None, group="allParams", advanced=False, invalidate=True, | ||||||||||||||||||
| semantic="", enabled=True, visible=True, exposed=True): | ||||||||||||||||||
| super(File, self).__init__(name=name, label=label, description=description, value=value, group=group, | ||||||||||||||||||
| advanced=advanced, enabled=enabled, invalidate=invalidate, semantic=semantic, | ||||||||||||||||||
|
|
@@ -325,6 +345,8 @@ def validateValue(self, value): | |||||||||||||||||
| 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 self.value is None: | ||||||||||||||||||
| return "", ValueTypeErrors.NONE | ||||||||||||||||||
| if not isinstance(self.value, str) and not callable(self.value): | ||||||||||||||||||
| return self.name, ValueTypeErrors.TYPE | ||||||||||||||||||
| return "", ValueTypeErrors.NONE | ||||||||||||||||||
|
|
@@ -333,7 +355,7 @@ def checkValueTypes(self): | |||||||||||||||||
| class BoolParam(Param): | ||||||||||||||||||
| """ | ||||||||||||||||||
| """ | ||||||||||||||||||
| def __init__(self, name, label, description, value, keyable=False, keyType=None, | ||||||||||||||||||
| def __init__(self, name, label=None, description=None, value=None, keyable=False, keyType=None, | ||||||||||||||||||
| group="allParams", advanced=False, enabled=True, invalidate=True, | ||||||||||||||||||
| semantic="", visible=True, exposed=False): | ||||||||||||||||||
| super(BoolParam, self).__init__(name=name, label=label, description=description, value=value, | ||||||||||||||||||
|
|
@@ -354,6 +376,8 @@ def validateValue(self, value): | |||||||||||||||||
| f"value: {value}, type: {type(value)})") | ||||||||||||||||||
|
|
||||||||||||||||||
| def checkValueTypes(self): | ||||||||||||||||||
| if self.value is None: | ||||||||||||||||||
| return "", ValueTypeErrors.NONE | ||||||||||||||||||
| if not isinstance(self.value, bool): | ||||||||||||||||||
| return self.name, ValueTypeErrors.TYPE | ||||||||||||||||||
| return "", ValueTypeErrors.NONE | ||||||||||||||||||
|
|
@@ -362,7 +386,7 @@ def checkValueTypes(self): | |||||||||||||||||
| class IntParam(Param): | ||||||||||||||||||
| """ | ||||||||||||||||||
| """ | ||||||||||||||||||
| def __init__(self, name, label, description, value, range=None, keyable=False, keyType=None, | ||||||||||||||||||
| def __init__(self, name, label=None, description=None, value=None, range=None, keyable=False, keyType=None, | ||||||||||||||||||
| group="allParams", advanced=False, enabled=True, invalidate=True, semantic="", | ||||||||||||||||||
| validValue=True, errorMessage="", visible=True, exposed=False): | ||||||||||||||||||
| self._range = range | ||||||||||||||||||
|
|
@@ -384,6 +408,8 @@ def validateValue(self, value): | |||||||||||||||||
| f"{value}, type: {type(value)})") | ||||||||||||||||||
|
|
||||||||||||||||||
| def checkValueTypes(self): | ||||||||||||||||||
| if self.value is None: | ||||||||||||||||||
| return "", ValueTypeErrors.NONE | ||||||||||||||||||
| if not isinstance(self.value, int): | ||||||||||||||||||
| return self.name, ValueTypeErrors.TYPE | ||||||||||||||||||
| if (self.range and not all([isinstance(r, int) for r in self.range])): | ||||||||||||||||||
|
|
@@ -396,7 +422,7 @@ def checkValueTypes(self): | |||||||||||||||||
| class FloatParam(Param): | ||||||||||||||||||
| """ | ||||||||||||||||||
| """ | ||||||||||||||||||
| def __init__(self, name, label, description, value, range=None, keyable=False, keyType=None, | ||||||||||||||||||
| def __init__(self, name, label=None, description=None, value=None, range=None, keyable=False, keyType=None, | ||||||||||||||||||
| group="allParams", advanced=False, enabled=True, invalidate=True, semantic="", | ||||||||||||||||||
| validValue=True, errorMessage="", visible=True, exposed=False): | ||||||||||||||||||
| self._range = range | ||||||||||||||||||
|
|
@@ -417,6 +443,8 @@ def validateValue(self, value): | |||||||||||||||||
| f"{value}, type:{type(value)})") | ||||||||||||||||||
|
|
||||||||||||||||||
| def checkValueTypes(self): | ||||||||||||||||||
| if self.value is None: | ||||||||||||||||||
| return "", ValueTypeErrors.NONE | ||||||||||||||||||
| if not isinstance(self.value, float): | ||||||||||||||||||
| return self.name, ValueTypeErrors.TYPE | ||||||||||||||||||
| if (self.range and not all([isinstance(r, float) for r in self.range])): | ||||||||||||||||||
|
|
@@ -429,7 +457,7 @@ def checkValueTypes(self): | |||||||||||||||||
| class PushButtonParam(Param): | ||||||||||||||||||
| """ | ||||||||||||||||||
| """ | ||||||||||||||||||
| def __init__(self, name, label, description, group="allParams", advanced=False, enabled=True, | ||||||||||||||||||
| def __init__(self, name, label=None, description=None, group="allParams", advanced=False, enabled=True, | ||||||||||||||||||
| invalidate=True, semantic="", visible=True, exposed=False): | ||||||||||||||||||
| super(PushButtonParam, self).__init__(name=name, label=label, description=description, value=None, | ||||||||||||||||||
| group=group, advanced=advanced, enabled=enabled, invalidate=invalidate, | ||||||||||||||||||
|
|
@@ -466,7 +494,7 @@ class ChoiceParam(Param): | |||||||||||||||||
| _OVERRIDE_SERIALIZATION_KEY_VALUE = "__ChoiceParam_value__" | ||||||||||||||||||
| _OVERRIDE_SERIALIZATION_KEY_VALUES = "__ChoiceParam_values__" | ||||||||||||||||||
|
|
||||||||||||||||||
| def __init__(self, name: str, label: str, description: str, value, values, exclusive=True, saveValuesOverride=False, | ||||||||||||||||||
| def __init__(self, name, label=None, description=None, value=None, values=None, exclusive=True, saveValuesOverride=False, | ||||||||||||||||||
| group="allParams", joinChar=" ", advanced=False, enabled=True, invalidate=True, semantic="", | ||||||||||||||||||
| validValue=True, errorMessage="", | ||||||||||||||||||
| visible=True, exposed=False): | ||||||||||||||||||
|
Comment on lines
+497
to
500
|
||||||||||||||||||
|
|
@@ -482,12 +510,14 @@ def __init__(self, name: str, label: str, description: str, value, values, exclu | |||||||||||||||||
| if self._values: | ||||||||||||||||||
| # Look at the type of the first element of the possible values | ||||||||||||||||||
| self._valueType = type(self._values[0]) | ||||||||||||||||||
| elif not exclusive: | ||||||||||||||||||
| elif not exclusive and self._value is not None: | ||||||||||||||||||
| # Possible values may be defined later, so use the value to define the type. | ||||||||||||||||||
| # if non exclusive, it is a list | ||||||||||||||||||
| self._valueType = type(self._value[0]) | ||||||||||||||||||
|
Comment on lines
515
to
516
|
||||||||||||||||||
| # if non exclusive, it is a list | |
| self._valueType = type(self._value[0]) | |
| # if non exclusive, it is a list; use the first element when available | |
| if self._value: | |
| self._valueType = type(self._value[0]) | |
| else: | |
| # Empty selection: element type cannot be inferred yet | |
| self._valueType = None |
Copilot
AI
Feb 21, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ChoiceParam.checkValueTypes() returns early when _value is None, which means it no longer validates that _values is a list. As a result, misconfigured descriptors (e.g. values=None or a non-list) won't be flagged by checkValueTypes() even though they can break validation/conformance later. Consider still validating _values (or explicitly allowing None with a dedicated error/behavior).
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,4 +1,7 @@ | ||||||
| from meshroom.core.graph import Graph | ||||||
| from meshroom.core import desc as coreDesc | ||||||
| from meshroom.core.desc.attribute import _labelFromName, StringParam, IntParam, FloatParam, BoolParam, File, ListAttribute, GroupAttribute | ||||||
|
||||||
| from meshroom.core.desc.attribute import _labelFromName, StringParam, IntParam, FloatParam, BoolParam, File, ListAttribute, GroupAttribute | |
| from meshroom.core.desc.attribute import _labelFromName, StringParam, IntParam, FloatParam, BoolParam, File |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getDefaultValue()usesself._desc._valueType()whendesc.value is None. Fordesc.ChoiceParamwithexclusive=False,_valueTypeis the element type (e.g.str), so this produces"", which then gets validated into[""]for the attribute default. This breaks the expected empty-list default for non-exclusive ChoiceParams and can lead to incorrect CLI formatting/serialization behavior. Consider special-casing non-exclusiveChoiceParamto default to an empty list (and add a regression test).