Don't include requirements only needed for example pipeline #3425

SajidAlamQB · 2023-12-14T12:20:54Z

Description

context: #3386

Development notes

Updated utils.py to remove seaborn and scikit-learn and the extras from kedro-datasets
Updated requirements and toml to Path parameter types
Updated release notes

Developer Certificate of Origin

We need all contributions to comply with the Developer Certificate of Origin (DCO). All commits must be signed off by including a Signed-off-by line in the commit message. See our wiki for guidance.

If your PR is blocked due to unsigned commits, then you must follow the instructions under "Rebase the branch" on the GitHub Checks page for your PR. This will retroactively add the sign-off to all unsigned commits and allow the DCO check to pass.

Checklist

Read the contributing guidelines
Signed off each commit with a Developer Certificate of Origin (DCO)
Opened this PR as a 'Draft Pull Request' if it is work-in-progress
Updated the documentation to reflect the code changes
Added a description of this change in the RELEASE.md file
Added tests to cover my changes
Checked if this change will affect Kedro-Viz, and if so, communicated that with the Viz team

Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>

merelcht · 2023-12-15T15:12:27Z

kedro/templates/project/hooks/utils.py

+        _remove_from_file(requirements_file_path, example_pipeline_requirements)
+        _remove_extras_from_kedro_datasets(requirements_file_path)


We also need to do this when there's no example chosen, but also no pyspark or viz right? https://github.com/kedro-org/kedro-starters/blob/main/spaceflights-pandas/%7B%7B%20cookiecutter.repo_name%20%7D%7D/requirements.txt

When we don't select pyspark or viz and no example is selected we get the template from kedro framework, I don't think that has any of the extra requirements so it doesn't need this logic.

We only use the spaceflights-pandas starter when we do want the example and don't include pyspark or viz so there isn't a case where we need to remove the requirements from here.

Ah yes you're right!

kedro/templates/project/hooks/utils.py

DimedS · 2023-12-15T16:10:26Z

kedro/templates/project/hooks/utils.py

@@ -15,6 +15,9 @@
 # Configuration key for documentation dependencies
 docs_pyproject_requirements = ["project.optional-dependencies"]  # For pyproject.toml

+# Requirements for example pipelines
+example_pipeline_requirements = "seaborn~=0.12.1\nscikit-learn~=1.0\n"


This approach seems somewhat risky, as we are hardcoding the complete version names for all the libraries slated for deletion. I'm concerned about how we will remember to update these if we decide to modify the version requirements in the future. This is also for lines 8-12.

Valid point I had the same feelings but we'll need to look for a more robust way to do it, maybe with regex?

I can create a follow up issue on this.

I agree, we need to think more about it, issue is a good point 👍

Done.
#3432

DimedS

Thank you for PR, @SajidAlamQB . Great job! Left few minor comments.

Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>

kedro/templates/project/hooks/utils.py

Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>

Signed-off-by: Sajid Alam <90610031+SajidAlamQB@users.noreply.github.com>

merelcht

LGTM! 👍

merelcht · 2023-12-18T11:49:00Z

kedro/templates/project/hooks/utils.py

+        _remove_from_file(requirements_file_path, example_pipeline_requirements)
+        _remove_extras_from_kedro_datasets(requirements_file_path)


Ah yes you're right!

* remove example pipeline requirements Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com> * lint Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com> * simplify amending kedro[...] lines Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com> * keep the version for datasets Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com> * lint Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com> --------- Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com> Signed-off-by: Sajid Alam <90610031+SajidAlamQB@users.noreply.github.com> Signed-off-by: Ahdra Merali <ahdra.merali@quantumblack.com>

* unique tool entires and QoL for pyproject.toml Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com> Signed-off-by: Ahdra Merali <ahdra.merali@quantumblack.com> * revert unique entries change Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com> Signed-off-by: Ahdra Merali <ahdra.merali@quantumblack.com> * Update cli.py template (#3428) Signed-off-by: Ahdra Merali <ahdra.merali@quantumblack.com> * Add logging about not using async for Sequential and Parallel runners (#3424) Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com> Signed-off-by: Ahdra Merali <ahdra.merali@quantumblack.com> * Don't include requirements only needed for example pipeline (#3425) * remove example pipeline requirements Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com> * lint Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com> * simplify amending kedro[...] lines Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com> * keep the version for datasets Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com> * lint Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com> --------- Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com> Signed-off-by: Sajid Alam <90610031+SajidAlamQB@users.noreply.github.com> Signed-off-by: Ahdra Merali <ahdra.merali@quantumblack.com> * Sort `requirements.txt` based on package name only (#3436) * Sort `requirements.txt` based on package name only Signed-off-by: Deepyaman Datta <deepyaman.datta@utexas.edu> * Remove now-unused custom `requirements.txt` sorter Signed-off-by: Deepyaman Datta <deepyaman.datta@utexas.edu> * Ruff format kedro/templates/project/hooks/utils.py Signed-off-by: Deepyaman Datta <deepyaman.datta@utexas.edu> * Pass the right argument to `fix_requirements` call Signed-off-by: Deepyaman Datta <deepyaman.datta@utexas.edu> * Add `sort_requirements` until starters are updated Signed-off-by: Deepyaman Datta <deepyaman.datta@utexas.edu> * Wrap lib call in existing method for compatibility Signed-off-by: Deepyaman Datta <deepyaman.datta@utexas.edu> --------- Signed-off-by: Deepyaman Datta <deepyaman.datta@utexas.edu> Signed-off-by: Ahdra Merali <ahdra.merali@quantumblack.com> * Pass tools through as list Signed-off-by: Ahdra Merali <ahdra.merali@quantumblack.com> * Revert "Pass tools through as list" This reverts commit f4a4a15. Signed-off-by: Ahdra Merali <ahdra.merali@quantumblack.com> * Remove duplicates Signed-off-by: Ahdra Merali <ahdra.merali@quantumblack.com> * Fix for no add-ons selected Signed-off-by: Ahdra Merali <ahdra.merali@quantumblack.com> * Lint Signed-off-by: Ahdra Merali <ahdra.merali@quantumblack.com> * Add example as recognised key Signed-off-by: Ahdra Merali <ahdra.merali@quantumblack.com> * Add example as recognised key pt2 Signed-off-by: Ahdra Merali <ahdra.merali@quantumblack.com> * Add example as recognised key pt3 Signed-off-by: Ahdra Merali <ahdra.merali@quantumblack.com> * Add example as recognised key pt4 Signed-off-by: Ahdra Merali <ahdra.merali@quantumblack.com> * Add example as recognised key pt5 Signed-off-by: Ahdra Merali <ahdra.merali@quantumblack.com> * Add example as recognised key pt6 Signed-off-by: Ahdra Merali <ahdra.merali@quantumblack.com> * Add example as recognised key pt7 Signed-off-by: Ahdra Merali <ahdra.merali@quantumblack.com> * Streamline condition Signed-off-by: Ahdra Merali <ahdra.merali@quantumblack.com> --------- Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com> Signed-off-by: Ahdra Merali <ahdra.merali@quantumblack.com> Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com> Signed-off-by: Sajid Alam <90610031+SajidAlamQB@users.noreply.github.com> Signed-off-by: Deepyaman Datta <deepyaman.datta@utexas.edu> Co-authored-by: Ahdra Merali <90615669+AhdraMeraliQB@users.noreply.github.com> Co-authored-by: Merel Theisen <49397448+merelcht@users.noreply.github.com> Co-authored-by: Deepyaman Datta <deepyaman.datta@utexas.edu> Co-authored-by: Ahdra Merali <ahdra.merali@quantumblack.com>

remove example pipeline requirements

a734628

Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>

SajidAlamQB self-assigned this Dec 14, 2023

SajidAlamQB requested a review from merelcht as a code owner December 14, 2023 12:20

lint

f735a42

Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>

SajidAlamQB requested a review from AhdraMeraliQB December 14, 2023 12:29

SajidAlamQB linked an issue Dec 14, 2023 that may be closed by this pull request

Don't include requirements only needed for example pipeline #3386

Closed

merelcht reviewed Dec 15, 2023

View reviewed changes

SajidAlamQB requested review from DimedS and merelcht December 15, 2023 15:37

Merge branch 'main' into dev/remove-requirements-when-no-examples

d9ea55b

DimedS reviewed Dec 15, 2023

View reviewed changes

kedro/templates/project/hooks/utils.py Outdated Show resolved Hide resolved

DimedS reviewed Dec 15, 2023

View reviewed changes

DimedS approved these changes Dec 15, 2023

View reviewed changes

simplify amending kedro[...] lines

599ccbc

Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>

DimedS reviewed Dec 15, 2023

View reviewed changes

kedro/templates/project/hooks/utils.py Outdated Show resolved Hide resolved

SajidAlamQB added 2 commits December 15, 2023 16:44

keep the version for datasets

33a47a6

Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>

lint

7d243d4

Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>

SajidAlamQB mentioned this pull request Dec 15, 2023

Avoid hardcoding requirements in project templates hooks utils.py #3432

Open

Merge branch 'main' into dev/remove-requirements-when-no-examples

926120e

Signed-off-by: Sajid Alam <90610031+SajidAlamQB@users.noreply.github.com>

merelcht approved these changes Dec 18, 2023

View reviewed changes

SajidAlamQB merged commit 51b47bf into main Dec 18, 2023

SajidAlamQB deleted the dev/remove-requirements-when-no-examples branch December 18, 2023 12:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Don't include requirements only needed for example pipeline #3425

Don't include requirements only needed for example pipeline #3425

SajidAlamQB commented Dec 14, 2023 •

edited

Loading

merelcht Dec 15, 2023

SajidAlamQB Dec 15, 2023 •

edited

Loading

merelcht Dec 18, 2023

DimedS Dec 15, 2023

SajidAlamQB Dec 15, 2023

SajidAlamQB Dec 15, 2023

DimedS Dec 15, 2023

SajidAlamQB Dec 15, 2023

DimedS left a comment

merelcht left a comment

merelcht Dec 18, 2023

		_remove_from_file(requirements_file_path, example_pipeline_requirements)
		_remove_extras_from_kedro_datasets(requirements_file_path)

Don't include requirements only needed for example pipeline #3425

Don't include requirements only needed for example pipeline #3425

Conversation

SajidAlamQB commented Dec 14, 2023 • edited Loading

Description

Development notes

Developer Certificate of Origin

Checklist

Choose a reason for hiding this comment

SajidAlamQB Dec 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DimedS left a comment

Choose a reason for hiding this comment

merelcht left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SajidAlamQB commented Dec 14, 2023 •

edited

Loading

SajidAlamQB Dec 15, 2023 •

edited

Loading