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

Implement the ItemsView protocol for component instances. Closes #237. #241

Merged
merged 1 commit into from
Apr 8, 2021

Conversation

AdamHillier
Copy link
Contributor

This implements most of ItemsView for components, which means that components can be iterated through, yielding pairs of (field_name, field_value). Importantly, this view is a flattened representation including the fields of all sub-components.

As a result, dict(component_instance) is a meaningful construction. To illustrate, here is an example of dict(component_instance) for a component with a nested sub-component:

{
    "a": 10, # Normal field
    "b": (5, -42.3), # Normal field
    "child": "namespace.filename.Child", # Component field - the qualified name of the class that's being used
    "child.b": (-1, 0.0), # Normal field (of sub-component)
}

In particular, this dict is the precise, minimal configuration needed to completely re-create the component, e.g. by doing:

config_dict = dict(old_instance)

new_instance = ComponentClass()
configure(new_instance, config_dict)

The above will work even if default field values have changed since the old instance was created.

Comment on lines 321 to 322
if hasattr(component_cls, "__len__"):
raise TypeError("Component classes must not define a custom `__len__` method.")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically this is a breaking change, but I strongly doubt that anybody was using component classes with custom __len__, __contains__, or __iter__ methods.

I'm happy to make a new minor version release but I think we can be slightly flexible about semver here and not worry about a new major version.

@@ -495,7 +595,7 @@ def configure_component_instance(
# If the field explicitly allows values to be missing, there's no need
# to do anything.
elif field.allow_missing:
pass
continue
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a bug, now fixed.

@AdamHillier AdamHillier marked this pull request as ready for review April 6, 2021 08:45
@AdamHillier AdamHillier requested a review from a team April 6, 2021 08:46
@AdamHillier AdamHillier added feature New feature or request internal-improvement labels Apr 6, 2021
Copy link
Member

@lgeiger lgeiger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! This is great 🚀

@lgeiger lgeiger merged commit d0d397e into larq:master Apr 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request internal-improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants