Custom props #37
Custom props #37
Conversation
| if "ref" in params: | ||
| data["ref"] = params.pop("ref") |
There was a problem hiding this comment.
@JonStargaryen @midlik Added support for refs as well which will be required to make the primitives works when showing lines/etc between different structures.
There was a problem hiding this comment.
I guess you wanted to say "Please add" instead of "Added"
There was a problem hiding this comment.
Well, partially added it myself :)
| if "ref" in params: | ||
| data["ref"] = params.pop("ref") |
There was a problem hiding this comment.
I guess you wanted to say "Please add" instead of "Added"
| custom: CustomT = Field(description="Custom data to store attached to this node.") | ||
| ref: RefT = Field(description="Optional reference that can be used to access this node.") |
There was a problem hiding this comment.
Wouldn't it be simpler to just include "custom" and "ref" as one of the node params? It would be defined in a params base class that all param classes would extend.
Instead of for example:
class ComponentInlineParams(BaseModel):
selector: Union[ComponentSelectorT, ComponentExpression, list[ComponentExpression]] = Field(...)
We would have:
class BaseParams(BaseModel):
custom: ...
ref: ...
class ComponentInlineParams(BaseParams):
selector: Union[ComponentSelectorT, ComponentExpression, list[ComponentExpression]] = Field(...)
I believe this will simplify code in Node.__init__ and make_params as they won't need to treat "custom" and "ref" in any special way.
There was a problem hiding this comment.
What would be the benefit of this other than simpler params?
There was a problem hiding this comment.
Although, you are right, this would also allow to not have ref/custom on all nodes for example if we ever don't want to support it everywhere.
|
Gonna merge this so I can test this in the primitives PR. Let's do other tweaks (like adding it to the builder everywhere in a separate pass) |
No description provided.