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

Add documentation about pyproject.toml #2427

Merged
merged 10 commits into from
Mar 22, 2023
Merged

Add documentation about pyproject.toml #2427

merged 10 commits into from
Mar 22, 2023

Conversation

ankatiyar
Copy link
Contributor

NOTE: Kedro datasets are moving from kedro.extras.datasets to a separate kedro-datasets package in
kedro-plugins repository. Any changes to the dataset implementations
should be done by opening a pull request in that repository.

Description

Resolves #2377

Development notes

I've added a section about the settings contained in pyproject.toml to the "Project Settings" (docs/source/kedro_project_setup/settings.md) page in "Kedro Project Setup" section

  • the link I removed in the docs/source/get_started/kedro_concepts.md isn't rendered as a link
  • updated project_version to kedro_init_version in the faqs

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

@ankatiyar
first draft pyproject.toml
20c9b57 
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
@datajoely
Copy link
Contributor

kedro_init_version is a much better name

astrojuanlu
astrojuanlu reviewed Mar 16, 2023
View reviewed changes
Copy link
Member

@astrojuanlu astrojuanlu left a comment

Choose a reason for hiding this comment

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

Thanks a lot @ankatiyar!

First comment that comes to mind: maybe we could use 3 different words for 3 different things:

I'm not a native speaker but, even though understanding that "configuration" and "settings" are mostly the same, and that pyproject.toml does indeed contain some "Kedro settings", by at least striving for internal consistency in the terminology we can hopefully minimize confusion.

@datajoely
Copy link
Contributor

image

I made this a while ago when were designing settings.py which is a Django inspired concept IIRC

@astrojuanlu
Copy link
Member

I love this @datajoely. Might inform @merelcht 's gh-2421

@astrojuanlu astrojuanlu mentioned this pull request Mar 17, 2023
Closed
@astrojuanlu
Copy link
Member

Let's continue the conversation about terminology in #2434.

@astrojuanlu
Copy link
Member

RTD build because of:

/home/docs/checkouts/readthedocs.org/user_builds/kedro/checkouts/2427/docs/kedro_project_setup/settings.md:8: WARNING: 'myst' reference target not found: #settings-in-pyproject.toml
/home/docs/checkouts/readthedocs.org/user_builds/kedro/checkouts/2427/docs/kedro_project_setup/settings.md:29: WARNING: Could not lex literal_block as "toml". Highlighting skipped.

astrojuanlu
astrojuanlu requested changes Mar 17, 2023
View reviewed changes
Copy link
Member

@astrojuanlu astrojuanlu left a comment

Choose a reason for hiding this comment

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

Added one suggestion about the wording, but other than that it's great having this finally, thanks @ankatiyar! 🙌🏽

docs/source/kedro_project_setup/settings.md Outdated Show resolved Hide resolved
docs/source/kedro_project_setup/settings.md Outdated Show resolved Hide resolved
stichbury
stichbury approved these changes Mar 17, 2023
View reviewed changes
Copy link
Contributor

@stichbury stichbury left a comment

Choose a reason for hiding this comment

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

Nice work -- thank you! Just a few minor tweaks 🏆 🏅

ankatiyar and others added 2 commits March 20, 2023 13:30
@ankatiyar @astrojuanlu
Update docs/source/kedro_project_setup/settings.md
41cd709 
Co-authored-by: Juan Luis Cano Rodríguez <juan_cano@mckinsey.com>
@ankatiyar @stichbury
Apply suggestions from code review
47a1605 
Co-authored-by: Jo Stichbury <jo_stichbury@mckinsey.com>
ankatiyar
ankatiyar commented Mar 20, 2023
View reviewed changes
docs/source/faq/faq.md Outdated Show resolved Hide resolved
ankatiyar and others added 2 commits March 20, 2023 14:28
@ankatiyar
lint + fix rtd build
32128fc 
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
@ankatiyar
Merge branch 'main' into docs/pyproject-toml
75a36a5
@stichbury
Copy link
Contributor

@ankatiyar Just to say that I fixed a couple of merge issues as a result of my FAQ PR, which is now in main.

@ankatiyar
Copy link
Contributor Author

@ankatiyar Just to say that I fixed a couple of merge issues as a result of my FAQ PR, which is now in main.

Thanks @stichbury! 👍

noklam
noklam reviewed Mar 20, 2023
View reviewed changes
docs/source/kedro_project_setup/settings.md
Comment on lines +40 to +42
You can also use `pyproject.toml` to specify settings for functionalities such as [micro-packaging](../nodes_and_pipelines/micro_packaging.md).
You can also store the settings for the other tools you've used in your project, such as [`pytest` for automated testing](../development/automated_testing.md).
Consult the respective documentation for the tools you have used to check how you can configure the settings with the `pyproject.toml` file for your project.
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a wild idea, can we link our pyproject.toml as an example?

https://github.com/kedro-org/kedro/blob/main/pyproject.toml

noklam
noklam reviewed Mar 20, 2023
View reviewed changes
docs/source/kedro_project_setup/settings.md
Comment on lines +37 to +38
`kedro_init_version` specifies the version of Kedro the project was created with. When you upgrade to a newer Kedro version,
this value should also be updated.
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't it the version of the "starter"? Just double checking

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's the version of Kedro used to create the project. It's also there when you make a blank project with kedro new

@ankatiyar ankatiyar merged commit 471153d into main Mar 22, 2023
@ankatiyar ankatiyar deleted the docs/pyproject-toml branch March 22, 2023 12:45
jmholzer pushed a commit that referenced this pull request Mar 29, 2023
@ankatiyar @astrojuanlu
@stichbury @jmholzer
Add documentation about pyproject.toml (#2427)
b3de03c 
* first draft pyproject.toml

Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>

* Update docs/source/kedro_project_setup/settings.md

Co-authored-by: Juan Luis Cano Rodríguez <juan_cano@mckinsey.com>

* Apply suggestions from code review

Co-authored-by: Jo Stichbury <jo_stichbury@mckinsey.com>

* lint + fix rtd build

Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>

* Fix linting errors introduced by merge from FAQ changes

Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>

---------

Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
Co-authored-by: Juan Luis Cano Rodríguez <juan_cano@mckinsey.com>
Co-authored-by: Jo Stichbury <jo_stichbury@mckinsey.com>
Signed-off-by: Jannic Holzer <jannic.holzer@quantumblack.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stichbury stichbury stichbury approved these changes

@noklam noklam noklam left review comments

@astrojuanlu astrojuanlu astrojuanlu approved these changes

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

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

Successfully merging this pull request may close these issues.

Add/improve documentation about pyproject.toml (first draft by development team)
5 participants
@ankatiyar @datajoely @astrojuanlu @stichbury @noklam