You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
We don't currently allow Producers to set default parameters (1, 2). This was originally to prevent a default value from "subverting" the compute_input_fingerprint calculation, but I think we may be able to relax the checks and still including during fingerprinting.
We should be able to set field defaults safely to Artifact instances, as these will be discoverable by the .inputs property (which is what feeds into dependency tracking), however build/map parameter defaults alone won't be visible so should still be blocked.
One caveat: the @producer setup generates the fields from the function signature - in this case, perhaps when adding fields to __annotations__, we can convert the default value (which would be a python value) to an Artifact instance (using Artifact.cast) and then clear the defaults (build.__defaults__ = None; build.__kwdefaults__ = None).
The text was updated successfully, but these errors were encountered:
We don't currently allow
Producer
s to set default parameters (1, 2). This was originally to prevent a default value from "subverting" thecompute_input_fingerprint
calculation, but I think we may be able to relax the checks and still including during fingerprinting.We should be able to set field defaults safely to
Artifact
instances, as these will be discoverable by the.inputs
property (which is what feeds into dependency tracking), howeverbuild
/map
parameter defaults alone won't be visible so should still be blocked.One caveat: the
@producer
setup generates the fields from the function signature - in this case, perhaps when adding fields to__annotations__
, we can convert the default value (which would be a python value) to an Artifact instance (usingArtifact.cast
) and then clear the defaults (build.__defaults__ = None; build.__kwdefaults__ = None
).The text was updated successfully, but these errors were encountered: