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

Comparing specs using == is incorrect sometimes #314

Open
andyk opened this issue Mar 11, 2022 · 0 comments
Open

Comparing specs using == is incorrect sometimes #314

andyk opened this issue Mar 11, 2022 · 0 comments

Comments

@andyk
Copy link
Contributor

andyk commented Mar 11, 2022

We currently use == in to_registry() on many spec classes such as here in ComponentRun.to_registry():

assert spec == self.to_spec(), (
f"A component run spec with identifier '{self.identifier}' "
f"already exists in registry '{registry}' and differs from "
"the one being added. Use force=True to overwrite the "
"existing one."
)

This can be wrong because currently some fields in specs can be omitted when their values are None. E.g., the WebRegistry always returns an agent and environment field for ComponentRun specs, even when the value of those attributes is None.

Essentially, we currently allow any optional spec field to be either omitted from the spec OR included and set to None.

Since {x: None} == {} returns False in Python (and rightfully so!) we need a smarter way of testing specs for equality.

We should be using a hash of the specs, and in fact the identifier should probably be the hash for all spec types (as it is currently for ArgumentSet specs). This should be addressed by #253, which will probably be done as part of #250.

@andyk andyk moved this to Todo in Sprint 0.2.0 Mar 12, 2022
@andyk andyk removed this from Sprint 0.2.0 Mar 22, 2022
@andyk andyk moved this to Todo in Sprint 0.2.2 Mar 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Todo
Development

No branches or pull requests

1 participant