Skip to content
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

Add a name property to nodes #65

Open
dalonsoa opened this issue Feb 16, 2024 · 1 comment
Open

Add a name property to nodes #65

dalonsoa opened this issue Feb 16, 2024 · 1 comment

Comments

@dalonsoa
Copy link
Collaborator

dalonsoa commented Feb 16, 2024

This seems sensible. As part of this would it be sensible to stop using the class.name as part of the model behaviour and instead make that a default node attribute? I had made this poor decision super early on and since realising that it's not a good idea it has now become all over the place. Maybe it is a separate issue because it seems to me a reasonably large task and the documentation here would need to be updated.

Originally posted by @barneydobson in #60 (comment)

@barneydobson
Copy link
Collaborator

barneydobson commented Mar 12, 2024

Related to this - currently if someone adds a type_ in config that is not a subclass of Node, provided that node_type_override is a subclass, that is fine - and the Model object uses the value in type_ to distinguish that node. You can see this behaviour currently with type_: Foul, i.e., it is not a Node subclass but Model calls it separately in run.

However, this is not very flexible - as, because Foul is not added to the node register, other nodes do not recognise it with function calls such as push_distributed(..., of_type = ['Foul']).

I think it is sensible that the solution for this enables such behaviour as it is an easy way to get a high degree of customisation with minimal coding.

Identified by @RobinMaesPrior

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants