8000 fix: change `is_optional` with `has_default` by ZanSara · Pull Request #155 · deepset-ai/canals · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
This repository was archived by the owner on Nov 28, 2023. It is now read-only.

fix: change is_optional with has_default #155

Merged
merged 4 commits into from
Nov 13, 2023

Conversation

ZanSara
Copy link
Contributor
@ZanSara ZanSara commented Nov 8, 2023

In preparation for: #105


This PR changes InputSocket.is_optional into InputSocket.has_default. This change affects both the semantic of the field and the behavior of Pipeline.

Optional is no more used to signal that the respective inputs are not to be waited for. Now the way to mark inputs as "not to be waited for" is to give them a default value, irrespective of their type.

In short: before, a component that declared:

def run(self, value: Optional[int])

was signaling the Pipeline that value is not to be waited for. Now the way to send the same signal to Pipeline is:

def run(self, value: int = 100)

where 100 could be any value.


Note: this breaks a lot of tests. Fixing this implies modifications in the run() method that will be addressed in separate PRs.

@ZanSara ZanSara marked this pull request as draft November 8, 2023 11:58
@ZanSara ZanSara changed the base branch from main to improve-tests November 8, 2023 13:32
@ZanSara ZanSara changed the title Change is_optional with has_default fix: change is_optional with has_default Nov 8, 2023
@ZanSara ZanSara marked this pull request as ready for review November 10, 2023 11:09
@masci masci force-pushed the change-is-optional-with-has-default branch 2 times, most recently from 371a24b to 4ba6954 Compare November 13, 2023 17:07
@masci masci merged commit a4ac1f7 into improve-tests Nov 13, 2023
@masci masci deleted the change-is-optional-with-has-default branch November 13, 2023 17:14
masci added a commit that referenced this pull request Nov 14, 2023
* change
6606
 is_optional to has_default

* do not hardcode defaults

* remove deepcopy

---------

Co-authored-by: Massimiliano Pippi <mpippi@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Optional type used to indicate default values
2 participants
0