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

Support default values for component arguments #179

Closed
3 tasks
PhilippeMoussalli opened this issue Jun 1, 2023 · 0 comments · Fixed by #199
Closed
3 tasks

Support default values for component arguments #179

PhilippeMoussalli opened this issue Jun 1, 2023 · 0 comments · Fixed by #199
Assignees
Labels
Core Core framework

Comments

@PhilippeMoussalli
Copy link
Contributor

PhilippeMoussalli commented Jun 1, 2023

Problem Statement

Some components have many arguments associated with them. It is not always necessary for the end-user to define all of them and we might want to stick with default values. Currently this is not available and all the components require an argument.

This feature should enable users to set default values for certain components while defining the components. Those values can then be optionally passed during the pipeline definition.

Proposed Approach

We first define the default arguments at the component_spec level:

compnent_spec.yaml

name:component
description: Component that downloads images based on URLs
image: ghcr.io/ml6team/download_images:latest

consumes:
   ...

produces:
  ...

args:
  min_image_size: #mandatory argument
    description: Minimum size of the images.
    type: int
  timeout: #optional argument
    description: Maximum time (in seconds) to wait when trying to download an image 
    default: 3
    type: int

Then, at the step when we add and parse the arguments in component.py
we check if a default argument is provided, if so we set the optional field when adding the argument:

    def _add_and_parse_args(self):
        parser = argparse.ArgumentParser()
        component_arguments = self._get_component_arguments()

        for arg in component_arguments.values():
            # Input manifest is not required for loading component
            if arg.name == "input_manifest_path":
                input_required = False
            else:
                input_required = True
           
            if arg.default is not None:
               default = arg.default
            else: 
               default = None
            parser.add_argument(
                f"--{arg.name}",
                type=kubeflow2python_type[arg.type],
                required=input_required,
                default=default,
                help=arg.description,
            )

        return parser.parse_args()

Additionally, we need to make the necessary changes to the component_spec.py by adding an optional default field to the Argument dataclass as well as modifying the KubeflowComponentSpec as specified here to include default arguments.

Implementation Steps/Tasks

Potential Impact

Testing

Create a component with optional argument and test whether omitting the argument will return the default values. Also test if we can override the default values successfully.

Additional testing will have to be done when running kubeflow but difficult to test with tests (has to be done manually with a mock submitted pipeline).

Documentation

Extend the Args documentation with examples of default pipelines.

Feedback and Suggestions

Dependent features

None

Additional Notes

@PhilippeMoussalli PhilippeMoussalli converted this from a draft issue Jun 1, 2023
@PhilippeMoussalli PhilippeMoussalli self-assigned this Jun 1, 2023
@PhilippeMoussalli PhilippeMoussalli added the Core Core framework label Jun 5, 2023
@PhilippeMoussalli PhilippeMoussalli moved this from Breakdown to In Progress in Fondant development Jun 12, 2023
@PhilippeMoussalli PhilippeMoussalli linked a pull request Jun 12, 2023 that will close this issue
@PhilippeMoussalli PhilippeMoussalli moved this from In Progress to Validation in Fondant development Jun 12, 2023
PhilippeMoussalli added a commit that referenced this issue Jun 13, 2023
PR that enables adding default arguments as discussed in #179. 

Users can now define default arguments in the component specs. Those
arguments do not have to be explicitly defined in the `ComponentOp` but
could still be overridden.

Note: please review this
[PR](#196) beforehand since I am
branched off from it.

Created a separate ticket to do the necessary changes #198. Best to
handle it in a separate PR.

---------

Co-authored-by: Robbe Sneyders <robbe.sneyders@gmail.com>
@github-project-automation github-project-automation bot moved this from Validation to Done in Fondant development Jun 13, 2023
Hakimovich99 pushed a commit that referenced this issue Oct 16, 2023
PR that enables adding default arguments as discussed in #179. 

Users can now define default arguments in the component specs. Those
arguments do not have to be explicitly defined in the `ComponentOp` but
could still be overridden.

Note: please review this
[PR](#196) beforehand since I am
branched off from it.

Created a separate ticket to do the necessary changes #198. Best to
handle it in a separate PR.

---------

Co-authored-by: Robbe Sneyders <robbe.sneyders@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core Core framework
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant