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

Mention the ability to disable output connections #433

Merged
merged 1 commit into from
Aug 2, 2024

Conversation

gpdf
Copy link
Contributor

@gpdf gpdf commented Aug 2, 2024

Checklist

  • ran Jenkins
  • added a release note for user-visible changes to doc/changes

Reviewer: please advise on whether "necessary" or "recommended" is more correct in:

"It is now no longer ( necessary / recommended ) to delegate to super in either case,"

There was clearly a word missing in any event.

@TallJimbo
Copy link
Member

Delegation to super is now guaranteed to be a no-op, so "recommended" is probably better.

Copy link

codecov bot commented Aug 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.22%. Comparing base (5079685) to head (1029755).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #433   +/-   ##
=======================================
  Coverage   80.22%   80.22%           
=======================================
  Files          96       96           
  Lines       10975    10975           
  Branches     2099     2099           
=======================================
  Hits         8805     8805           
  Misses       1823     1823           
  Partials      347      347           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Note that the same mechanism works for output connections:
an output from a `PipelineTask` can be made optional (under config control)
or a config switch can even be used to choose between different ways of
providing output.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth clarifying that this is different from making an input optional at runtime by setting minimum=0 in the Input connection declaration. In the configuration case, the connection is treated as if it never existed by QG generation and execution code, while minimum=0 allows a quantum to be generated and run even no dataset for that input are found. (There is no minimum for Output connections because they are always considered runtime-optional.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know if you think I've captured it now.

@gpdf gpdf force-pushed the u/gpdf/optional-outputs-1 branch from a599c83 to 1029755 Compare August 2, 2024 17:25
@gpdf gpdf force-pushed the u/gpdf/optional-outputs-1 branch from 1029755 to 31565d2 Compare August 2, 2024 22:10
@gpdf
Copy link
Contributor Author

gpdf commented Aug 2, 2024

Squashed commits in preparation for merge

@gpdf gpdf merged commit 89d1b78 into main Aug 2, 2024
11 checks passed
@gpdf gpdf deleted the u/gpdf/optional-outputs-1 branch August 2, 2024 22:12
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

Successfully merging this pull request may close these issues.

2 participants