-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[ui/core] Add Validators paradigm #3088
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 |
|---|---|---|
|
|
@@ -12,6 +12,7 @@ | |
| from collections.abc import Iterable, Sequence | ||
| from string import Template | ||
| from meshroom.common import BaseObject, Property, Variant, Signal, ListModel, DictModel, Slot | ||
| from meshroom.core.desc.validators import NotEmptyValidator | ||
| from meshroom.core import desc, hashValue | ||
| from meshroom.core.keyValues import KeyValues | ||
| from meshroom.core.exception import InvalidEdgeError | ||
|
|
@@ -84,6 +85,7 @@ def __init__(self, node, attributeDesc: desc.Attribute, isOutput: bool, root=Non | |
| self._enabled: bool = True | ||
| self._depth: int = root.depth + 1 if root is not None else 0 | ||
| self._exposed: bool = root.exposed if root is not None else attributeDesc.exposed | ||
| self._description: str = attributeDesc.description | ||
| self._invalidate = False if self._isOutput else attributeDesc.invalidate | ||
| self._invalidationValue = "" # invalidation value for output attributes | ||
| self._value = None | ||
|
|
@@ -235,6 +237,15 @@ def _setValue(self, value): | |
| self.requestNodeUpdate() | ||
| self.valueChanged.emit() | ||
|
|
||
| def _get_description(self): | ||
| return self._description | ||
|
|
||
| def _set_description(self, desc): | ||
| if self._description == desc: | ||
| return | ||
| self._description = desc | ||
| self.descriptionChanged.emit() | ||
|
|
||
| def _getKeyValues(self): | ||
| """ | ||
| Return the per-key values object of the attribute or of the linked attribute. | ||
|
|
@@ -401,21 +412,6 @@ def _isDefault(self): | |
| else: | ||
| return self._getValue() == self.getDefaultValue() | ||
|
|
||
| def _isValid(self): | ||
| """ | ||
| Check attribute description validValue: | ||
| - If it is a function, execute it and return the result | ||
| - Otherwise, simply return true | ||
| """ | ||
| if callable(self._desc.validValue): | ||
| try: | ||
| return self._desc.validValue(self.node) | ||
| except Exception as exc: | ||
| if not self.node.isCompatibilityNode: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can't see a corresponding check on the new code ? |
||
| logging.warning(f"Failed to evaluate 'isValid' (node lambda) for attribute '{self.fullName}': {exc}") | ||
| return True | ||
| return True | ||
|
|
||
| def _is2dDisplayable(self) -> bool: | ||
| """ | ||
| Return True if the current attribute is considered as a displayable 2D file. | ||
|
|
@@ -487,6 +483,43 @@ def updateInternals(self): | |
| # Emit if the enable status has changed | ||
| self._setEnabled(self._getEnabled()) | ||
|
|
||
| def getErrorMessages(self) -> list[str]: | ||
| """ Execute the validators and aggregate the eventual error messages""" | ||
|
|
||
| result = [] | ||
|
|
||
| for validator in self.desc.validators: | ||
| isValid, errorMessages = validator(self.node, self) | ||
|
|
||
| if isValid: | ||
| continue | ||
|
|
||
| for errorMessage in errorMessages: | ||
| result.append(errorMessage) | ||
|
|
||
| return result | ||
|
|
||
| def _isValid(self) -> bool: | ||
| """ Check the validation and return False if any validator return (False, erorrs) | ||
| """ | ||
|
|
||
|
nicolas-lambert-tc marked this conversation as resolved.
|
||
| for validator in self.desc.validators: | ||
| isValid, _ = validator(self.node, self) | ||
|
|
||
| if not isValid: | ||
| return False | ||
|
|
||
| return True | ||
|
|
||
| def _isMandatory(self) -> bool: | ||
| """ An attribute is considered as mandatory it contain a NotEmptyValidator """ | ||
|
|
||
| for validator in self.desc.validators: | ||
| if isinstance(validator, NotEmptyValidator): | ||
| return True | ||
|
|
||
| return False | ||
|
|
||
| def _getEnabled(self) -> bool: | ||
| if callable(self._desc.enabled): | ||
| try: | ||
|
|
@@ -742,6 +775,9 @@ def validateIncomingConnection(self, connectingAttribute: Attribute) -> bool: | |
|
|
||
| expressionApplied = Signal() | ||
|
|
||
| errorMessageChanged = Signal() | ||
| errorMessages = Property(Variant, lambda self: self.getErrorMessages(), notify=errorMessageChanged) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can't see that signal emitted ?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's needed because it's used internally by Qml. If not set, an error occure:
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't we have some caching here ? I guess getErrorMessages() is called many times, even if the attribute value did not change.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried to use caching, but I failed trying to keep the same robustness. Problems with connect/disconnect, undo/redo |
||
| isMandatory = Property(bool, _isMandatory, constant=True ) | ||
|
|
||
| def raiseIfLink(func): | ||
| """ | ||
|
|
||
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| from typing import TYPE_CHECKING, Protocol, runtime_checkable | ||
|
|
||
| if TYPE_CHECKING: | ||
| from meshroom.core.attribute import Attribute | ||
| from meshroom.core.node import Node | ||
|
|
||
|
|
||
| def success() -> tuple[bool, list[str]]: | ||
| return (True, []) | ||
|
|
||
| def error(*messages: str) -> tuple[bool, list[str]]: | ||
| return (False, list(messages)) | ||
|
|
||
| @runtime_checkable | ||
| class AttributeValidator(Protocol): | ||
| """ Interface for an attribute validation | ||
| You can inherit from this class and override the __call__ methods to implement your own attribute validation logic | ||
|
|
||
| Because it's a callable class, you can also create your own validators on the fly | ||
|
|
||
| .. code-block: python | ||
|
|
||
| lambda node, attribute: success() if attribute.value and attribute.value != "" else error("attribute have no value") | ||
| """ | ||
|
|
||
| def __call__(self, node: "Node", attribute: "Attribute") -> tuple[bool, list[str]]: | ||
| """ | ||
| Override this method to implement your custom validation logic. | ||
| You can use the success() and error() helpers that encapsulate the returning response. | ||
|
|
||
| :param node: The node that holds the attribute to validate | ||
| :param attribute: The atribute to validate | ||
|
|
||
| :returns: The validtion response: (True, []) if it's valid, (False, [errorMessage1, errorMessage2, ...]) if error exists | ||
|
|
||
| """ | ||
| raise NotImplementedError() | ||
|
|
||
|
|
||
| class NotEmptyValidator(AttributeValidator): | ||
| """ The attribute value should not be empty | ||
| This class is used to determine if an attribute label should be considered as mandatory/required | ||
| """ | ||
|
|
||
| def __call__(self, node: "Node", attribute: "Attribute") -> tuple[bool, list[str]]: | ||
|
|
||
| if attribute.value is None or attribute.value == "": | ||
| return error("Empty value is not allowed") | ||
|
|
||
| return success() | ||
|
|
||
|
|
||
| class RangeValidator(AttributeValidator): | ||
| """ Check if the attribute value is in the given range | ||
| """ | ||
|
|
||
| def __init__(self, min, max): | ||
| self._min = min | ||
| self._max = max | ||
|
|
||
| def __call__(self, node:"Node", attribute: "Attribute") -> tuple[bool, list[str]]: | ||
|
|
||
| if attribute.value < self._min or attribute.value > self._max: | ||
| return error(f"Value should be greater than {self._min} and less than {self._max}", | ||
| f"({self._min} < {attribute.value} < {self._max})") | ||
|
|
||
| return success() |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1522,6 +1522,8 @@ def _onAttributeChanged(self, attr: Attribute): | |
| if callback: | ||
| callback(self) | ||
|
|
||
| self.hasInvalidAttributeChanged.emit() | ||
|
|
||
| if self.graph: | ||
| # If we are in a graph, propagate the notification to the connected output attributes | ||
| for edge in self.graph.outEdges(attr): | ||
|
|
@@ -1792,7 +1794,7 @@ def loadOutputAttr(self): | |
| # This does not apply to non dynamic output | ||
| if not self.nodeDesc.hasDynamicOutputAttribute: | ||
| return | ||
|
|
||
| # Check existence of values.json file | ||
| valuesFile = self.valuesFile | ||
| if not os.path.exists(valuesFile): | ||
|
|
@@ -2160,6 +2162,12 @@ def hasTextOutputAttribute(self) -> bool: | |
| """ | ||
| return next((attr for attr in self._attributes if attr.enabled and attr.isOutput and attr.isTextDisplayable), None) is not None | ||
|
|
||
| def _hasInvalidAttribute(self): | ||
| for attribute in self._attributes: | ||
| if len(attribute.errorMessages) > 0: | ||
| return True | ||
| return False | ||
|
|
||
| def _hasDisplayableShape(self): | ||
| """ | ||
|
Comment on lines
2171
to
2172
|
||
| Return True if at least one attribute is a ShapeAttribute, a ShapeListAttribute or a shape File. | ||
|
|
@@ -2240,6 +2248,9 @@ def _hasDisplayableShape(self): | |
| # Whether the node contains a ShapeAttribute, a ShapeListAttribute or a shape File. | ||
| hasDisplayableShape = Property(bool, _hasDisplayableShape, constant=True) | ||
|
|
||
| hasInvalidAttributeChanged = Signal() | ||
| hasInvalidAttribute = Property(bool, _hasInvalidAttribute, notify=hasInvalidAttributeChanged) | ||
|
|
||
|
|
||
| class Node(BaseNode): | ||
| """ | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.