-
Notifications
You must be signed in to change notification settings - Fork 59
Handle-none-outputs #790
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
base: main
Are you sure you want to change the base?
Handle-none-outputs #790
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #790 +/- ##
==========================================
+ Coverage 88.51% 88.67% +0.15%
==========================================
Files 83 83
Lines 17719 17751 +32
Branches 3441 3451 +10
==========================================
+ Hits 15684 15740 +56
+ Misses 1665 1642 -23
+ Partials 370 369 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
4e5d4ab
to
7aaa539
Compare
…uts are set to be empty
7aaa539
to
aa35926
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to me, good tests. One small thing I noticed that could be addressed here or later.
if inputs is not None: | ||
if not isinstance(inputs, dict): | ||
raise ValueError( | ||
f"Input names ({inputs}) should not be provided when " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just looking at this, it seems like inputs
could be a list[str | Arg]
, but it isn't turned into a dict before this. This means we'll now error on an empty list as well as a populated list, but maybe we should just remove the list annotation?
Types of changes
Summary
Handles the case of tasks and workflows without any outputs (i.e. return type is set to None). In this case the outputs are empty.
Checklist