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

[Ready] Docs changes to remove pandas-iris and update kedro new flow in onboarding docs #3317

Merged
merged 45 commits into from
Nov 27, 2023

Conversation

stichbury
Copy link
Contributor

@stichbury stichbury commented Nov 16, 2023

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

To address #3048 by updating the new project creation flow.

Ready for first review by @amandakys.

PREVIEW CODE IS BUILT HERE

Development notes

I have updated the new project text but have also:

  • split the starters documentation into two sections for usage and starter creation, both are tidied up for language and content
  • created a new folder for docs about starters and what I've currently titled "new project tools"
  • revised text throughout the docs that refers to pandas-iris

Still to come

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: Jo Stichbury <jo_stichbury@mckinsey.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
@stichbury
Copy link
Contributor Author

stichbury commented Nov 20, 2023

Some notes on what we need to do to complete this PR:

  • Update the get started section to introduce the new project creation flow that uses the different options / remove Iris starter etc. @stichbury to do this
  • Add a page devoted to the different options for the new project creation flow. I've called it "Addons" right now but that needs to be changed (page name, titles etc). We seem to have settled on "Tools" although I'm not massively confident in how to use the terminology so hope @AhdraMeraliQB you can write the page and see how many adjustments are needed. It's coming in this PR Add tools documentation page #3331
  • Revision to the page about Kedro starters (for usage) @AhdraMeraliQB can you take this too please?
  • Introduction of a new page (in the "Extend Kedro" section) which takes the old content about how to create a starter and updates it as needed. @stichbury to do the first cut of this.
  • Review the rest of the docs where kedro new is discussed and adjust (particularly to remove the Iris starter and add tools options). See Revise documentation that uses the iris starter #3160 for a list, which I've worked through except for one query about generators (requested input from @noklam and @merelcht)

stichbury and others added 10 commits November 21, 2023 15:23
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
```bash
kedro new --config=<path>/config.yml
```
The configuration file must contain:

- `output_dir` The path in which to create the project directory
- `project_name`
- `repo_name`
- `python_package`
Copy link
Contributor

@AhdraMeraliQB AhdraMeraliQB Nov 22, 2023

Choose a reason for hiding this comment

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

Add-ons are also configurable here - will double check what happens when they are omitted

Update: The process fails if they are omitted, consistent with the other attributes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I'll need some insight on how to include these so will await the changes on your branch for guidance.

Ahdra Merali added 2 commits November 22, 2023 12:05
Signed-off-by: Ahdra Merali <ahdra.merali@quantumblack.com>
Signed-off-by: Ahdra Merali <ahdra.merali@quantumblack.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
docs/source/faq/faq.md Outdated Show resolved Hide resolved
@astrojuanlu
Copy link
Member

I gave a look and looks okay! I see there are some parts yet to be written still

stichbury and others added 5 commits November 23, 2023 13:57
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
Co-authored-by: Juan Luis Cano Rodríguez <juan_luis_cano@mckinsey.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
@stichbury
Copy link
Contributor Author

This is ready for final review @astrojuanlu and @merelcht @amandakys

The sections marked as "to do" will be covered by @AhdraMeraliQB and @SajidAlamQB in a separate PR (branched from this branch) here #3331

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.

Looks great! I've left some minor comments.

docs/source/nodes_and_pipelines/nodes.md Outdated Show resolved Hide resolved
docs/source/starters/create_a_starter.md Outdated Show resolved Hide resolved
docs/source/starters/starters.md Outdated Show resolved Hide resolved
docs/source/starters/starters.md Show resolved Hide resolved
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.

Gave this a more thorough review. The content is fantastic - I have a few reservations about the overall structure of the starters folder. I made some specific suggestions in the comments and opened some questions.

@@ -0,0 +1,44 @@
# New project tools
Copy link
Member

Choose a reason for hiding this comment

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

It's weird that /starters/ is for "new project tools", /starters/starters.html is for "starters", and /starters/new_project_tools.html is for "configure a new project" (looking at the H1 of each page).

Copy link
Member

Choose a reason for hiding this comment

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

And speaking specifically about this title, "new project tools" could be read as "tools for new projects" or "new thing called project tools", which was confusing to me at first. I bet we don't want to use the second meaning, given that the "new" tools will stop being new at some point...

Copy link
Contributor Author

@stichbury stichbury Nov 27, 2023

Choose a reason for hiding this comment

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

I think we should fix the naming in the upcoming PR from @AhdraMeraliQB which explicitly writes the content. I can't really guess at what to call the page right now, so that's a placeholder.

You're correct about the /starters being an odd place for those tools but it's awkward to name the folder. Maybe "/project_creation. Again, I'm pushing that to #3331 rather than resolving here.

docs/source/starters/index.md Outdated Show resolved Hide resolved
docs/source/index.rst Show resolved Hide resolved
docs/source/extend_kedro/index.md Show resolved Hide resolved
docs/source/starters/index.md Show resolved Hide resolved
docs/source/starters/starters.md Show resolved Hide resolved
docs/source/starters/starters.md Show resolved Hide resolved
@amandakys
Copy link

I looked at the New Project Tools page and the Create a new Kedro project pages.

I like the way the two pages link together, the Create a New Kedro Project page is now much clearer and flows well.

On the New Project Tools page, would it be worth highlighting the kedro new --addons flag since we are listing out flags to use with the kedro new command? Or are we documenting that elsewhere?

On the Create a New Kedro Project. Would it worth having a section called Project Name? That way the in page navigation will reflect the structure of the CLI. Otherwise, we could signpost that there are 3 steps to the kedro new command at the top and list the steps before we go into more detail?

stichbury and others added 5 commits November 27, 2023 10:23
Co-authored-by: Merel Theisen <49397448+merelcht@users.noreply.github.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
Co-authored-by: Merel Theisen <49397448+merelcht@users.noreply.github.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
@stichbury stichbury merged commit 01e6929 into develop Nov 27, 2023
11 checks passed
@stichbury stichbury deleted the fix-starters-content branch November 27, 2023 13:40
stichbury added a commit that referenced this pull request Nov 28, 2023
…low in onboarding docs (#3317)

* Revise link to notebook docs and remove unnecessary intro page

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

* Update starters content

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

* relocate starters content

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

* Added some changes for add-ons and some to do notes

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

* Some further fixes

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

* Move section about development version of Kedro

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

* Add text for new project

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

* Remove mention of pandas-iris where possible, replacing with alternative

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

* Fix linter errors

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

* Update new project docs

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

* Remove deprecated starters from architecture diagram

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

* Add warning for pandas-iris usage in generator section

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

* Further updates for instances of kedro new

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

* Remove TODO as no longer required

Signed-off-by: Ahdra Merali <ahdra.merali@quantumblack.com>

* Resolve some Vale issues and remove implication of tools + starters

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

* fixes to internal links

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

* pandas-spaceflights bad, spaceflights-pandas good

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

* fix cookiecutter docs urls

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

* Update the create a starter docs

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

* Fix link to avoid linkcheck barf

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

* Update docs/source/get_started/new_project.md

Co-authored-by: Juan Luis Cano Rodríguez <juan_luis_cano@mckinsey.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>

* Update following review

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

* Update content

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

* Update docs/source/nodes_and_pipelines/nodes.md

Co-authored-by: Merel Theisen <49397448+merelcht@users.noreply.github.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>

* Update docs/source/starters/starters.md

Co-authored-by: Merel Theisen <49397448+merelcht@users.noreply.github.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>

* Update FAQ

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

* Updates following review

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

---------

Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
Signed-off-by: Ahdra Merali <ahdra.merali@quantumblack.com>
Co-authored-by: Ahdra Merali <90615669+AhdraMeraliQB@users.noreply.github.com>
Co-authored-by: Ahdra Merali <ahdra.merali@quantumblack.com>
Co-authored-by: Juan Luis Cano Rodríguez <juan_luis_cano@mckinsey.com>
Co-authored-by: Merel Theisen <49397448+merelcht@users.noreply.github.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
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.

6 participants