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

What is the desired behaviour when input_spec isn't provided to Workflow __init__? #491

Closed
tclose opened this issue Aug 18, 2021 · 7 comments

Comments

@tclose
Copy link
Contributor

tclose commented Aug 18, 2021

pydra/pydra/engine/core.py

Lines 796 to 817 in e0ea60d

if input_spec:
if isinstance(input_spec, BaseSpec):
self.input_spec = input_spec
else:
self.input_spec = SpecInfo(
name="Inputs",
fields=[("_graph_checksums", ty.Any)]
+ [
(
nm,
attr.ib(
type=ty.Any,
metadata={
"help_string": f"{nm} input from {name} workflow"
},
),
)
for nm in input_spec
],
bases=(BaseSpec,),
)
self.output_spec = output_spec

If an input_spec isn't provided to Workflow.init what should happen, an empty input_spec generated? Because as it stands self.input_spec isn't set and the method gets stuck in an infinite loop trying to access the attribute that isn't there.

@djarecka
Copy link
Collaborator

@tclose - thanks for reporting this! we should raise an exception. Would you be interested in creating PR?

@tclose
Copy link
Contributor Author

tclose commented Aug 30, 2021

Sure, do you want an empty input_spec created in the case when it isn't provided?

@djarecka
Copy link
Collaborator

No, I think we want to have an error, we do want to have at least one input for every single workflow and discourage people from hard-coding input paths etc. Unless, you have an example that you can show it will be useful

@tclose
Copy link
Contributor Author

tclose commented Sep 21, 2021

#497 addresses this issue

@tclose
Copy link
Contributor Author

tclose commented Sep 21, 2021

One thing that seems a little counterintuitive to me unless I don't understand how it works (quite likely) is the ability to define both the input_spec and input values in the Workflow init.

I was expecting a Workflow to conceptually map onto a task (i.e. an "uninstantiated node", sorry not sure what nomeclature you use), which then could be parameterised for different applications. However, it seems that you are parameterising it as it is defined, do I have that right?

@djarecka
Copy link
Collaborator

I think the main reason for input_spec was that we allow for input provided as kwarg to Workflow, but perhaps we might rething

@tclose
Copy link
Contributor Author

tclose commented Oct 6, 2021

This is now addressed by #497

@tclose tclose closed this as completed Oct 6, 2021
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

2 participants