Update demo project to use OmegaConfigLoader #1590

aanghelidi · 2023-10-19T05:28:00Z

Description

Development notes

To migrate the project to use OmegaConfigLoader, the migration guide was followed. During this migration, I encountered some small issues that I have fixed:

A deprecated parameter of the machine learning model in the pipeline. This was changed to the new default.
Several warnings about DataSet to be renamed Dataset fixed by the renaming.
Fix a Pandas warning, SettingWithCopyWarning. This was fixed using loc to slice a subset of the DataFrame in the pipeline.

Note: I have renamed demo-project/src/demo_project/requirements.in to demo-project/src/dev_requirements.txt but this is optional and could be reverted

QA notes

To test my changes, the following steps were performed:

Making sure I could install the dependencies, following Makefile and instructions in README
Run pipeline with kedro run
Run existing tests with pytest

Checklist

Read the contributing guidelines
Opened this PR as a 'Draft Pull Request' if it is work-in-progress
Updated the documentation to reflect the code changes
Added new entries to the RELEASE.md file
Added tests to cover my changes

Screenshot

noklam · 2023-10-19T12:24:25Z

demo-project/src/docker_requirements.txt

 kedro-datasets[pandas.CSVDataSet,pandas.ExcelDataSet, pandas.ParquetDataSet, plotly.PlotlyDataSet, matplotlib.MatplotlibWriter]~=1.0
-scikit-learn~=1.0
+kedro>=0.18.14, <0.19.0
 pillow~=9.0
+scikit-learn~=1.0
 seaborn~=0.11.2


Question to @ravi-kumar-pilla or @rashidakanchwala, what are these two dev_requirements.txt and docker_requirements.txt for? I am not sure if it's appropriate to have -r docker_requirements.txt.

Is 0.18.14 the minimum version or the 1st version with dataset factory?

Based on my understanding, requirements.in (dev_requirements.txt) is for local dev dependencies while docker_requirements.txt are used when demo-project is deployed (Step Build demo container image in .circleci/continue_config.yml), one of the steps for deploy_demo to aws ecr.

I think there is no issue in referencing docker_requirements in dev_requirements. I could see seaborn to be an additional requirement in dev which was not present earlier.

0.18.14 is the latest version of kedro right now which has most of the OmegaConfigLoader features. I guess that was the rationale behind the update. Happy to hear from @rashidakanchwala

Initial PR when demo_project was created for reference

noklam · 2023-10-19T12:25:28Z

demo-project/conf/base/catalog_04_feature.yml

+"feature_engineering.feat_{metric_type}_metrics":
+ type: pandas.ParquetDataset
+ filepath: ${_base_location}/04_feature/feat_{metric_type}_metrics.pq
+ layer: feature
+
+feature_importance_output:
+  type: pandas.CSVDataset
+  filepath: ${_base_location}/04_feature/feature_importance_output.csv
  metadata:


Great use of the datasets factory 👍🏼

demo-project/conf/prod/globals.yml

noklam · 2023-10-24T12:23:33Z

We had a call last week and IRRC the conclusion is we want to keep the requirements separately instead of having it relies on the docker requirements.

merelcht

This PR is introducing a lot more changes than the original issue kedro-org/kedro#3172 described. I really appreciate your efforts here @aanghelidi, but this many changes in one PR makes it harder to review and as flagged by the team members not every change is actually desired, e.g. the changes to how requirements are handled.

Would you mind updating this PR and only keep:

The changes to use OmegaConfigLoader
The use of dataset factories
A deprecated parameter of the machine learning model in the pipeline. This was changed to the new default.
Fix a Pandas warning, SettingWithCopyWarning. This was fixed using loc to slice a subset of the DataFrame in the pipeline.

Thank you!

demo-project/pyproject.toml

ravi-kumar-pilla

Thank you for the PR @aanghelidi. Looks good to me and works well in local testing. But, I would agree with @merelcht on keeping the desired changes here. Thank you

aanghelidi · 2023-10-27T04:46:37Z

Thank you for the thorough review @merelcht @ravi-kumar-pilla @noklam and @ankatiyar I appreciated the feedback. Only the desired changes and improvements have been kept for this PR answering what the original issue kedro-org/kedro#3172 described.

The only exception is the addition of seaborn (identical to docker requirements) in the development requirements as mentioned by @ravi-kumar-pilla, because the pipeline won't run without it.

merelcht

Thanks a lot @aanghelidi ! These changes look good to me now ⭐ 👍

Question for the viz team: should this change go into the release notes for our own record?

tynandebold · 2023-10-27T09:33:20Z

I think we could add a line to the release notes for this. Good thought!

demo-project/conf/base/catalog_08_reporting.yml

ravi-kumar-pilla

Thank you @aanghelidi for the PR. Apart from the typo in README.md file, everything else looks good to me and works well.

ravi-kumar-pilla · 2023-10-27T15:40:49Z

demo-project/README.md

-1. Run `pip install kedro==0.18.4`
-2. Run `kedro install --build-reqs`
+1. Run `pip install kedro~=0.18.0`
+2. Run `pip install -r src/demo-project/requirements.in`


[Typo in demo_project] Can you please change the command to pip install -r src/demo_project/requirements.in

Good catch ! The command has been changed to pip install -r src/demo_project/requirements.in

demo-project/conf/base/catalog_08_reporting.yml

Signed-off-by: Alain Anghelidi <alainanghelidi@gmail.com>

Since the version 1.1 of scikit learn the default value of the parameter max_features of RandomForestRegressor have been changed from 'auto' to 1.0. Support for the old 'auto' value used have been removed. This commit fix this issue. Signed-off-by: Alain Anghelidi <alainanghelidi@gmail.com>

Signed-off-by: Alain Anghelidi <alainanghelidi@gmail.com>

tynandebold · 2023-10-30T12:09:35Z

We can add a release note at a later date. I'm going to merge this now.

This is minor release with big backend refactoring work and some bug fixes. Bug fixes and other changes Refactor flowchart dataclasses to pydantic base models. (Refactor Flowchart models from dataclass to pydantic base models #1565) Fix dataset factory patterns in Experiment Tracking. (Fix dataset factory patterns in Experiment Tracking #1588) Update demo-project to use OmegaConfigLoader. (Update demo project to use OmegaConfigLoader #1590) Improve feedback for copy to clipboard feature. (Add tooltip to shareable urls copy button #1614) Ensure Kedro-Viz works when hosted on a URL subpath. (Fix: Kedro-Viz doesn't work when hosted via a URL subpath #1621) Bump fastapi upper bounds. (Bump FAST API upper bounds #1634) Fix shareable URL modal to appear across the app. (Make shareable URL modal open globally across the app. #1639) Add Kedro-Viz CLI command deprecation warning. (Add kedro viz deprecation warning for CLI #1641)

NeroOkwa · 2023-11-17T17:32:14Z

Thank you @aanghelidi for this PR. Here's the release announcement on the Kedro slack acknowledging your contribution.

aanghelidi changed the title ~~Docs/update demo project to omega config loader~~ Update demo project to omega config loader Oct 19, 2023

aanghelidi changed the title ~~Update demo project to omega config loader~~ Update demo project to use OmegaConfigLoader Oct 19, 2023

tynandebold requested review from merelcht and noklam October 19, 2023 09:26

tynandebold added Python Pull requests that update Python code Community labels Oct 19, 2023

noklam reviewed Oct 19, 2023

View reviewed changes

aanghelidi force-pushed the docs/update-demo-project-to-omega-config-loader branch from c25cd3a to 539d501 Compare October 19, 2023 20:20

aanghelidi marked this pull request as ready for review October 19, 2023 20:25

aanghelidi requested review from tynandebold, yetudada and rashidakanchwala as code owners October 19, 2023 20:25

ankatiyar reviewed Oct 20, 2023

View reviewed changes

demo-project/conf/prod/globals.yml Outdated Show resolved Hide resolved

aanghelidi force-pushed the docs/update-demo-project-to-omega-config-loader branch from 539d501 to a266be2 Compare October 21, 2023 08:25

tynandebold requested review from noklam, ankatiyar and ravi-kumar-pilla October 24, 2023 11:52

merelcht requested changes Oct 24, 2023

View reviewed changes

demo-project/pyproject.toml Outdated Show resolved Hide resolved

ravi-kumar-pilla reviewed Oct 24, 2023

View reviewed changes

tynandebold removed the request for review from yetudada October 25, 2023 11:12

aanghelidi force-pushed the docs/update-demo-project-to-omega-config-loader branch from a266be2 to 5cf549f Compare October 27, 2023 04:26

merelcht self-requested a review October 27, 2023 09:27

merelcht approved these changes Oct 27, 2023

View reviewed changes

ravi-kumar-pilla reviewed Oct 27, 2023

View reviewed changes

demo-project/conf/base/catalog_08_reporting.yml Show resolved Hide resolved

ravi-kumar-pilla approved these changes Oct 27, 2023

View reviewed changes

Add missing dependency

5264822

Signed-off-by: Alain Anghelidi <alainanghelidi@gmail.com>

aanghelidi added 7 commits October 28, 2023 14:15

Replace deprecated command in README

3fb39c4

Signed-off-by: Alain Anghelidi <alainanghelidi@gmail.com>

Bump kedro init version

abc3a01

Signed-off-by: Alain Anghelidi <alainanghelidi@gmail.com>

Use OmegaConfigLoader in project settings

513fcae

Signed-off-by: Alain Anghelidi <alainanghelidi@gmail.com>

Remove unneeded CONFIG_LOADER_ARGS

e2beb9e

Signed-off-by: Alain Anghelidi <alainanghelidi@gmail.com>

Update configuration to use OmegaConfigLoader

1ed9394

Signed-off-by: Alain Anghelidi <alainanghelidi@gmail.com>

Fix Pandas SettingWithCopyWarning by using loc

b790049

Signed-off-by: Alain Anghelidi <alainanghelidi@gmail.com>

aanghelidi force-pushed the docs/update-demo-project-to-omega-config-loader branch from 5cf549f to b790049 Compare October 28, 2023 12:16

tynandebold merged commit d25db88 into kedro-org:main Oct 30, 2023
5 checks passed

rashidakanchwala mentioned this pull request Nov 16, 2023

Release v6.7.0 #1642

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update demo project to use OmegaConfigLoader #1590

Update demo project to use OmegaConfigLoader #1590

aanghelidi commented Oct 19, 2023 •

edited

noklam Oct 19, 2023

ravi-kumar-pilla Oct 19, 2023

ravi-kumar-pilla Oct 19, 2023

noklam Oct 19, 2023

noklam commented Oct 24, 2023

merelcht left a comment •

edited

ravi-kumar-pilla left a comment

aanghelidi commented Oct 27, 2023

merelcht left a comment

tynandebold commented Oct 27, 2023

ravi-kumar-pilla left a comment

ravi-kumar-pilla Oct 27, 2023

aanghelidi Oct 28, 2023

tynandebold commented Oct 30, 2023

NeroOkwa commented Nov 17, 2023

Update demo project to use OmegaConfigLoader #1590

Update demo project to use OmegaConfigLoader #1590

Conversation

aanghelidi commented Oct 19, 2023 • edited

Description

Development notes

QA notes

Checklist

Screenshot

noklam Oct 19, 2023

Choose a reason for hiding this comment

ravi-kumar-pilla Oct 19, 2023

Choose a reason for hiding this comment

ravi-kumar-pilla Oct 19, 2023

Choose a reason for hiding this comment

noklam Oct 19, 2023

Choose a reason for hiding this comment

noklam commented Oct 24, 2023

merelcht left a comment • edited

Choose a reason for hiding this comment

ravi-kumar-pilla left a comment

Choose a reason for hiding this comment

aanghelidi commented Oct 27, 2023

merelcht left a comment

Choose a reason for hiding this comment

tynandebold commented Oct 27, 2023

ravi-kumar-pilla left a comment

Choose a reason for hiding this comment

ravi-kumar-pilla Oct 27, 2023

Choose a reason for hiding this comment

aanghelidi Oct 28, 2023

Choose a reason for hiding this comment

tynandebold commented Oct 30, 2023

NeroOkwa commented Nov 17, 2023

aanghelidi commented Oct 19, 2023 •

edited

merelcht left a comment •

edited