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

Redefining an entity with a function can produce surprising results #115

Open
jqmp opened this issue Apr 24, 2020 · 0 comments
Open

Redefining an entity with a function can produce surprising results #115

jqmp opened this issue Apr 24, 2020 · 0 comments

Comments

@jqmp
Copy link
Collaborator

jqmp commented Apr 24, 2020

When an already-existing entity is defined with a function, its previous declaration (including any protocol, docstring, etc.) is erased. This is intended behavior, but it can be surprising -- especially since it's only mentioned in passing in the docs. (It's also mentioned in the docstring of FlowBuilder.__call__, but that doesn't show up anywhere in our documentation.)

Here's a minimal test that a new user might expect to pass, but doesn't:

def test_declared_protocol_is_kept(builder):
    protocol = bn.protocol.dillable()
    builder.declare("n", protocol=protocol)

    @builder
    def n():
        return 0

    assert builder.build().entity_protocol("n") == protocol

There are a few ways we could improve this:

  1. Document this behavior more clearly.
  2. Have different decorator APIs for defining new entities vs redefining existing ones, like we do with assign and set. This would hopefully make it more obvious that the new definition completely erases the previous one.
  3. Change the semantics from "overwrite previous entity and create new one" to "change the value definition of an existing entity, but keep the previous protocol, docstring, etc."
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant