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

Update how to get pipelines for new kedro version #53

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

cshaley
Copy link
Contributor

@cshaley cshaley commented Jun 21, 2023

Description

Why was this PR created?
context.pipelines does not work in kedro 0.18

Development notes

What have you changed, and how has this been tested?
changed to use new find_pipelines function, tested locally

Checklist

  • Read the contributing guidelines
  • Open this PR as a 'Draft Pull Request' if it is work-in-progress
  • Update the documentation to reflect the code changes
  • Add a description of this change and add your name to the list of supporting contributions in the CHANGELOG.md file. Please respect Keep a Changelog guidelines.
  • Add tests to cover your changes

Notice

  • I acknowledge and agree that, by checking this box and clicking "Submit Pull Request":

  • I submit this contribution under the Apache 2.0 license and represent that I am entitled to do so on behalf of myself, my employer, or relevant third parties, as applicable.

  • I certify that (a) this contribution is my original creation and / or (b) to the extent it is not my original creation, I am authorised to submit this contribution on behalf of the original creator(s) or their licensees.

  • I certify that the use of this contribution as authorised by the Apache 2.0 license does not violate the intellectual property rights of anyone else.

@rxm7706
Copy link

rxm7706 commented Jun 24, 2023

@cshaley do we need a little clean up here - to get the workflow matrix matching the setup.py... maybe @elanqo can help here. Not sure if github actions will take care of it..

python_requires=">=3.8, <3.11",

python_requires=">=3.8, <3.11",
python-version: [3.6, 3.7]

python-version: [3.6, 3.7]
python-version: [3.7]

python-version: [3.7]

@@ -5,6 +5,7 @@
import numpy as np
import pandas as pd
from fastapi import FastAPI
from kedro.framework.project import find_pipelines
Copy link
Owner

Choose a reason for hiding this comment

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

@cshaley I think the current modification is not the right one. The find_pipelines() function looks for create_pipeline`() functions in the project template, but any pipeline filtered / modified /created in the pipeline.py will not be loaded. I think the right way to load kedro pipelines is now :

from kedro.framework.project import pipelines

@@ -21,6 +21,7 @@ def serving_commands():
@click.option(
"--pipeline",
"-n", # n for "name"
default="__default__",
Copy link
Owner

Choose a reason for hiding this comment

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

Actually I did not put a default value for the pipeline argument of the serve command on purpose. I think many pipelines (including the default one) cannot be served with kedro-serving because they don't have a single input. In most ML projects, the __default__ pipeline is a combination of training + predicting + computing some metrics, and there are very often other inputs than only the raw data. in your projects, do you often most times that the __default__ can be served?

@Galileo-Galilei
Copy link
Owner

@rxm7706 is right, the worklflow is quite old. I've updated it in kedro-pandera recently, we can likely copy-paste it but it can be another PR.

@cshaley : See my comments directly in the code

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.

3 participants