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

[Parent] - Add-ons Project Creation flow #3216

Closed
10 tasks done
noklam opened this issue Oct 23, 2023 · 7 comments
Closed
10 tasks done

[Parent] - Add-ons Project Creation flow #3216

noklam opened this issue Oct 23, 2023 · 7 comments

Comments

@noklam
Copy link
Contributor

noklam commented Oct 23, 2023

Milestone: https://github.com/kedro-org/kedro/milestone/33 (To see what's left)

Blocking (Top priority):

This issue is currently blocking all new add-ons Pr (i.e. spark, viz) because we have different template structure in kedro/templates and kedro-starters.

In Progress:

Blocked:

Design Ticket:

Final:

  • Let's do a final spike at the end to try our best to break this workflow
    Checking:
  • kedro new - Should trigger add-ons flow
  • kedro new --add-ons --starter should trigger an error, invalid combination of options
  • kedro new + select pyspark will pull from kedro-starters
  • Do kedro run to test that the created projects can be run successfully.
  • "Data Structure" - do we have a better name?
  • fill in the correct URLs to the relevant docs for each step of the flow

Potential Improvement:

  • Refactor the logic to a separate add_on.py? It started to feel it overcrowd the starters.py. Starters still exist as a separate feature but kedro new can use either starter flow or add_on
@amandakys
Copy link

I've gone for Data Folder in the prototype that i've updated in #3054

@noklam
Copy link
Contributor Author

noklam commented Oct 25, 2023

I notice that in this PR. @amandakys

kedro new --addons="test, docs, log"

Should pyproject.toml stores this or the other mappings? Should we even have two?

ADD_ONS_DICT = {
    "1": "Linting",
    "2": "Testing",
    "3": "Custom Logging",
    "4": "Documentation",
    "5": "Data Structure",
}

@SajidAlamQB
Copy link
Contributor

I notice that in this PR. @amandakys

I noticed it to, the ADD_ONS_DICT is used in the backend for post_gen_project.py while the other mappings are just for the cli. Is it worth aligning the naming between the backend and cli?

@amandakys
Copy link

amandakys commented Oct 25, 2023

I'm not sure i fully understand the problem but I think we agreed that the --addons flag would have the shortnames 'lint, docs, test' etc. While in the copy, they are called Linting, Testing, Documentation etc. For the user all they see is the copy, and in the --help they are told what the corresponding shortnames are.

I think it makes more sense to pick one set of names for the implementation, and the shortnames are probably fine. We can think of the long names as "display/presentation" names and the shortnames as like the "code" names

@noklam
Copy link
Contributor Author

noklam commented Oct 26, 2023

👍🏼, So pyproject.toml will keep the long name and CLI will keep the short. I can take the rest from here as it's just implementation details.

@noklam
Copy link
Contributor Author

noklam commented Nov 3, 2023

Just watch the video of the add-ons tech design.

I started some refactoring before watching the video, I share a lot of opinion with @DimedS.

Pure Function - @DimedS has a good point. IMO we shouldn't mutate function inputs unless it helps a lot on performance, which I think 99% of the case we shouldn't do it. Even if this has to be done, we should adopt some pattern. For example, in Pytorch there is an user interface add function, internally they have a add_ which actually mutate the tensor.

#3265 is a first step and it can be merged without interfering with existing works, the other refactor will affect PR that are still in progress so I prefer to do it a bit later.

@merelcht
Copy link
Member

merelcht commented Dec 6, 2023

All major tasks listed on this issue have been completed and we did the bug bash to discover small issues that will be addressed as lower priority.

@merelcht merelcht closed this as completed Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

No branches or pull requests

4 participants