Add type check #938
Merged
Add type check #938
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
remove schema validators for GCRPath, and adjust for GCRPath, GCSPath change _check_valid_dict to _check_valid_type_dict to avoid confusion fix typo in the comments adjust function order for readability
update the _check_valid_type_dict name
hongye-sun
reviewed
Mar 8, 2019
| for input_spec in component_spec.inputs: | ||
| if input_spec.name == key: | ||
| if arguments[key].param_type is not None and not check_types(arguments[key].param_type.to_dict_or_str(), '' if input_spec.type is None else input_spec.type): | ||
| raise InconsistentTypeException('Component "' + name + '" is expecting ' + key + ' to be type(' + str(input_spec.type) + '), but the passed argument is type(' + arguments[key].param_type.serialize() + ')') |
There was a problem hiding this comment.
Consider to refactor the args check code in a function which can be reused by component, python_component, and yaml component.
There was a problem hiding this comment.
I'm not sure it's possible in the current code architecture.
If the ContainerOp is backed by TaskSpec and ComponentSpec internally, then this would be possible as there would be a single place where the consistency is checked: the place where we construct TaskSpec from ComponentSpec + arguments. Basically, right this place.
P.S. AFAIK, python_component, and yaml component already reuse the same code.
Ark-kun
reviewed
Mar 8, 2019
Ark-kun
reviewed
Mar 8, 2019
|
One thing I've noticed pretty late is that you're using dictionaries to annotate the returns of functions. In Lightweight python components I've used NamedTuple for that which is the true return type in that case: Maybe later we can think about |
|
/lgtm |
Ark-kun
reviewed
Mar 8, 2019
|
/lgtm |
…bug; also fix the bug when there are not a single return annotations
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gaoning777 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
cheyang
pushed a commit
to alibaba/pipelines
that referenced
this pull request
Mar 28, 2019
* add core types and type checking function * fix unit test bug * avoid defining dynamic classes * typo fix * add component metadata format * add a construct for the component decorator * add default values for the meta classes * add input/output types to the metadata * add from_dict in TypeMeta * small fix * add unit tests * use python struct for the openapi schema * add default in parameter * add default value * remove the str restriction for the param default * bug fix * add pipelinemeta * add pipeline metadata * ignore annotation if it is not str/BaseType/dict * update param name in the check_type functions remove schema validators for GCRPath, and adjust for GCRPath, GCSPath change _check_valid_dict to _check_valid_type_dict to avoid confusion fix typo in the comments adjust function order for readability * remove default values for non-primitive types in the function signature update the _check_valid_type_dict name * pass metadata from component decorator and task factory to containerOp * pass pipeline metadata to Pipeline * fix unit test * typo in the comments * move the metadata classes to a separate module * fix unit test * small change * add __eq__ to meta classes not export _metadata classes * nothing * fix unit test * unit test python component * unit test python pipeline * fix bug: duplicate variable of args * fix unit tests * move python_component and _component decorator in _component file * remove the print * change parameter default value to None * add functools wraps around _component decorator * TypeMeta accept both str and dict * fix indent, add unit test for type as strings * do not set default value for the name field in ParameterMeta, ComponentMeta, and PipelineMeta * add type check in task factory * output error message * add type check in component decorator; move the metadata assignment out of the containerop __init__ function * fix bug; add unit test * add more unit tests * more unit tests; fix bugs * more unit tests; fix bugs * add unit tests * more unit tests * add type check switch; add unit tests * add compiler option for type check * resolving pr comments * add unit test for pipeline param check with component types; fix the bug; also fix the bug when there are not a single return annotations
Linchin
pushed a commit
to Linchin/pipelines
that referenced
this pull request
Apr 11, 2023
Add PULL_BASE_REF for checking out repos
magdalenakuhn17
pushed a commit
to magdalenakuhn17/pipelines
that referenced
this pull request
Oct 22, 2023
…kubeflow#938) * Fix logger port * Add logger e2e test * Fix cloud event type * Fix event display image
HumairAK
pushed a commit
to red-hat-data-services/data-science-pipelines
that referenced
this pull request
Mar 11, 2024
* upgrade tekton to 0.35.1 Upgrade tekton to 0.35.1, including: - tekton manifests - go.mod to use new pkg and update all related pkgs - update pipeline-loops to also use new tekton pkg Signed-off-by: Yihong Wang <yh.wang@ibm.com> * update tekton to 0.36 update tekton, license files, and cache server Signed-off-by: Yihong Wang <yh.wang@ibm.com> * fix compile error on test code for taskrun's step, it doesn't use container data struct any more Signed-off-by: Yihong Wang <yh.wang@ibm.com> * update compiler add `type` for the taskSpec.results and update all yaml files accordingly Signed-off-by: Yihong Wang <yh.wang@ibm.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add type check in the task factory
This change is