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.

Sign up for GitHub

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

Jump to bottom

Merge template pyproject.toml into one #2926

Merged
merged 9 commits into from
Oct 5, 2023
Merged

Merge template pyproject.toml into one #2926

merged 9 commits into from
Oct 5, 2023

Conversation

astrojuanlu
Copy link
Member

@astrojuanlu astrojuanlu commented Aug 14, 2023 •
edited by SajidAlamQB
Loading

Description

Continuation of #2853.

This merges the two pyproject.toml (essentially move one of them one directory up) to have only one project metadata file and turn the template into a Pythonic src-layout package 🎉

Also, moved the dependencies from src/requirements.txt to requirements.txt

Development notes

Warning
Moving tests and pyproject.toml outside of src does break backwards incompatibility for kedro package.
Moving requirements.txt out of src requires changes in docs, kedro-docker, and possibly other places.

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 a description of this change in the RELEASE.md file
  • Added tests to cover my changes

@astrojuanlu astrojuanlu force-pushed the breaking-merge-template-pyproject branch from f78e99e to 841d239 Compare August 16, 2023 16:22
@astrojuanlu astrojuanlu mentioned this pull request Aug 23, 2023
Closed
@DimedS DimedS mentioned this pull request Aug 25, 2023
Closed
@DimedS DimedS linked an issue Aug 25, 2023 that may be closed by this pull request
Merge pyproject.toml files for starters in line with python src-layout #2975
Closed
@astrojuanlu astrojuanlu force-pushed the breaking-merge-template-pyproject branch from d5f4ed7 to f721321 Compare September 1, 2023 17:27
@astrojuanlu astrojuanlu mentioned this pull request Sep 20, 2023
Merged
9 tasks
@astrojuanlu astrojuanlu force-pushed the breaking-merge-template-pyproject branch from f721321 to fe3f2e4 Compare September 20, 2023 15:42
@astrojuanlu astrojuanlu changed the base branch from main to develop September 20, 2023 15:42
@astrojuanlu astrojuanlu reopened this Sep 20, 2023
@astrojuanlu

This comment was marked as outdated.

Sign in to view
@astrojuanlu astrojuanlu marked this pull request as ready for review September 20, 2023 15:53
@astrojuanlu
Copy link
Member Author

This is ready for a first pass.

@astrojuanlu astrojuanlu changed the title Breaking merge template pyproject Merge template pyproject.toml into one Sep 20, 2023
@astrojuanlu astrojuanlu force-pushed the breaking-merge-template-pyproject branch 2 times, most recently from 09f3498 to 91bc011 Compare September 25, 2023 13:28
merelcht
merelcht reviewed Sep 25, 2023
View reviewed changes
Copy link
Member

@merelcht merelcht left a comment

Choose a reason for hiding this comment

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

Left some small comments, but generally this looks good!

features/steps/test_starter/{{ cookiecutter.repo_name }}/pyproject.toml Outdated Show resolved Hide resolved
features/steps/test_starter/{{ cookiecutter.repo_name }}/pyproject.toml Show resolved Hide resolved
kedro/framework/cli/project.py Outdated Show resolved Hide resolved
@astrojuanlu astrojuanlu force-pushed the breaking-merge-template-pyproject branch from 91bc011 to 69dee90 Compare September 26, 2023 20:09
@astrojuanlu astrojuanlu force-pushed the breaking-merge-template-pyproject branch 3 times, most recently from fc4bc1c to 796df75 Compare October 2, 2023 17:58
@astrojuanlu
Copy link
Member Author

@merelcht I intend to iterate on the tests if the CI fails, but other than that I resolved all the outstanding comments

SajidAlamQB
SajidAlamQB reviewed Oct 4, 2023
View reviewed changes
Copy link
Contributor

@SajidAlamQB SajidAlamQB left a comment

Choose a reason for hiding this comment

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

Happy for this to go in once tests are all passing. Thank you @astrojuanlu!

@astrojuanlu
Turn default template into src-layout Python package
21082ac 
Signed-off-by: Juan Luis Cano Rodríguez <juan_luis_cano@mckinsey.com>
@astrojuanlu
Fix tests after metadata and tests moved
2ed4d15 
Signed-off-by: Juan Luis Cano Rodríguez <juan_luis_cano@mckinsey.com>
@astrojuanlu
Remove leftover mention of environment.yml
a0b5005 
Signed-off-by: Juan Luis Cano Rodríguez <juan_luis_cano@mckinsey.com>
@astrojuanlu astrojuanlu force-pushed the breaking-merge-template-pyproject branch from 796df75 to f21779f Compare October 4, 2023 06:44
@astrojuanlu

This comment was marked as resolved.

Sign in to view
@astrojuanlu astrojuanlu force-pushed the breaking-merge-template-pyproject branch 2 times, most recently from b11ca3e to 83c4e66 Compare October 4, 2023 09:22
@astrojuanlu
Move src/requirements.txt one level up
5dc6ec4 
Signed-off-by: Juan Luis Cano Rodríguez <juan_luis_cano@mckinsey.com>
@astrojuanlu
Make kedro package backwards compatible
82b6ec1 
Signed-off-by: Juan Luis Cano Rodríguez <juan_luis_cano@mckinsey.com>
@astrojuanlu
Add template changes to release notes
828a9f1 
Signed-off-by: Juan Luis Cano Rodríguez <juan_luis_cano@mckinsey.com>
@astrojuanlu astrojuanlu force-pushed the breaking-merge-template-pyproject branch from 83c4e66 to 828a9f1 Compare October 4, 2023 09:29
@astrojuanlu
Copy link
Member Author

All tests passing, now failing coverage 👍🏽 Will need to add one more test for kedro package with old template

@SajidAlamQB
Add test for coverage
7e7d8e9 
Signed-off-by: SajidAlamQB <90610031+SajidAlamQB@users.noreply.github.com>
@SajidAlamQB
lint
2e9d3a4 
Signed-off-by: SajidAlamQB <90610031+SajidAlamQB@users.noreply.github.com>
@SajidAlamQB
Fix outdated doc link
6b11116 
Signed-off-by: SajidAlamQB <90610031+SajidAlamQB@users.noreply.github.com>
merelcht
merelcht approved these changes Oct 5, 2023
View reviewed changes
Copy link
Member

@merelcht merelcht left a comment

Choose a reason for hiding this comment

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

Great work @astrojuanlu and thanks @SajidAlamQB for adding the missing test 👍 ⭐

SajidAlamQB
SajidAlamQB approved these changes Oct 5, 2023
View reviewed changes
Copy link
Contributor

@SajidAlamQB SajidAlamQB left a comment

Choose a reason for hiding this comment

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

Awesome work @astrojuanlu! 🌟

@astrojuanlu
Copy link
Member Author

Cannot approve my own PR 😄 thanks @SajidAlamQB for taking over 🙏🏽

@astrojuanlu astrojuanlu merged commit 2f526b6 into develop Oct 5, 2023
53 of 58 checks passed
@astrojuanlu astrojuanlu deleted the breaking-merge-template-pyproject branch October 5, 2023 10:18
This was referenced Oct 5, 2023
Closed
Closed
@SajidAlamQB SajidAlamQB mentioned this pull request Oct 25, 2023
Merged
3 tasks
@astrojuanlu astrojuanlu mentioned this pull request Feb 12, 2024
Closed
@astrojuanlu astrojuanlu mentioned this pull request Jun 27, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@merelcht merelcht merelcht approved these changes

@SajidAlamQB SajidAlamQB SajidAlamQB approved these changes

@yetudada yetudada Awaiting requested review from yetudada yetudada is a code owner

@stichbury stichbury Awaiting requested review from stichbury

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Merge pyproject.toml files for starters in line with python src-layout
3 participants
@astrojuanlu @merelcht @SajidAlamQB