-
Notifications
You must be signed in to change notification settings - Fork 54
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
Bugfix/handle default fields #96
Conversation
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.
Refactor using no unnecessary constants
def complete_with_default(self, data: OrderedDict) -> None: | ||
"""Add default data in place when fields are missing""" | ||
for key in NONE_OPTIONAL_FIELDS: | ||
if key not in data: | ||
raise ValueError(f"{key} of the socket is missing") | ||
|
||
for key in DEFAULT_BLOCK_DATA: | ||
if key not in data: | ||
data[key] = DEFAULT_BLOCK_DATA[key] |
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.
Using global constants lead to a triple duplicate of this same code, a good example of why constants should be use with care !
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.
I still feel like this is a duplicate, am I missing something ?
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.
I don't get it, doesn't the Serializable refactor clear the need of this branch anyway ?
def complete_with_default(self, data: OrderedDict) -> None: | ||
"""Add default data in place when fields are missing""" | ||
for key in NONE_OPTIONAL_FIELDS: | ||
if key not in data: | ||
raise ValueError(f"{key} of the socket is missing") | ||
|
||
for key in DEFAULT_BLOCK_DATA: | ||
if key not in data: | ||
data[key] = DEFAULT_BLOCK_DATA[key] |
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.
I still feel like this is a duplicate, am I missing something ?
Add default fields to edges, sockets, blocks, code blocks and scenes.
Resolves issue #95 for most practical purposes.