Minor tech debts in the project creation flow

[noklam]

Description

Clean out some tech debts in the new project creation flow. This ticket documents the issues I observed, it can be tackled in one PR or separately depending how many issues we found at the end.

Issue 1 -> ❌ won't be done: see discussion below

• 1. the type passing should be a list, it's str now but everything is functional because we use in everywhere so either list or str works. It makes more sense to keep it as a list[str] nonetheless.

Issue 2 -> fixed ✅ in #3429

• 2. Inconsistent logic updating cookiecutter arguments

In the kedro new interactive flow, the cookiecutter argument is set with this logic.

    # Map the selected add on lists to readable name
    add_ons = config.get("add_ons")
    if add_ons:
        config["add_ons"] = [
            ADD_ONS_DICT[add_on] for add_on in _parse_add_ons_input(add_ons)  # type: ignore
        ]
        config["add_ons"] = str(config["add_ons"])

While in kedro new --addon, we use another logic.

    if selected_addons is not None:
        addons = selected_addons.split(",")
        for i in range(len(addons)):
            addon = addons[i].strip()
            if addon in string_to_number:
                addons[i] = string_to_number[addon]
        return ",".join(addons)

Issue 3 -> addons will be renamed to tools, so this isn't an issue anymore. -> fixed ✅

• 3. Use of addon, add_on, add_ons. Particular, in the cli we use kedro new --addons, but pyproject.toml stores add_ons, we should keep this consistent.

Issue 4. -> fixed ✅

• The code isn't using best practices when it comes to keeping functions pure and not mutating input/output. Let's try and clean this up where possible.

Issue 5 - Not relevant ❌ , see this discussion

• Add exception handling for file and directory operations in case of issues like permission errors or files being used by another process.

Issue 6. -> fixed ✅ in #3429

• Looks like _validate_config_file_inputs function duplicates the prompt validation process described in Prompt.validate() function, but with additional hardcoded duplicated values from prompts.yml. The same could be also for _select_prompts_to_display() func. Or if we think that we need to check some 'kedro new' options syntax in any case without depending what written in prompts.yml I think we need to clarify that and hardcode only once in starters.py.

Issue 7 -> fixed fully ✅ in #3499

• Maybe we should consider to put default values of add_ons and example options when loading from config, not strictly check it with _validate_config_file_against_prompts().
  FIXED partly✅ in 3342
  We need to check are we providing defaults for tools and example_pipeline and is it a correct place?

Issue 8

• In starters.py in new() function use flag_inputs variable when process CLI inputs

Task List

• [Bug fix] - Make cookiecutter arguments consistent for all kedro new path #3266 - address 1.
• Make project creation flow parse add-ons consistently #3265 - address 2.
• Issue 5
• Complete Issue 7

[SajidAlamQB]

To add to this:

I think we should be adding exception handling for file and directory operations in case of issues like permission errors or files being used by another process.

[DimedS]

Thanks @noklam, I want to add few more to discuss:

1. Looks like _validate_config_file_inputs function duplicates the prompt validation process described in Prompt.validate() function, but with additional hardcoded duplicated values from prompts.yml. The same could be also for _select_prompts_to_display() func. Or if we think that we need to check some 'kedro new' options syntax in any case without depending what written in prompts.yml I think we need to clarify that and hardcode only once in starters.py.
2. Maybe we should consider to put default values of add_ons and example options when loading from config, not strictly check it with _validate_config_file_against_prompts().

[AhdraMeraliQB]

CC: @noklam

RE: Issue 1 - Attempted in #3426 but causes problems with cookiecutter as lists are handled as a list of options. To change the type here we'd need ton instead use a dictionary, which would affect a lot of the current logic. I'm of the opinion that it should be left as a string, what do you think?

[noklam]

@AhdraMeraliQB Let's keep it as str.

For the record there is an cookiecutter issue and I try to find some workaround without success, see cookiecutter/cookiecutter#1975 (comment).

Previous attempt PR: #3266

[AhdraMeraliQB]

Update:

Issue 5 - Already handled. xref
Issue 7 - Default values for tools and example_pipeline are provided when not included in config. See this PR and the developer docs
Issue 8 - I'm not sure I understand the problem to address here.

My suggestion: close the issue as completed. CC @noklam @DimedS @merelcht

[noklam]

Thanks @AhdraMeraliQB . I am closing it now, if it turns out the last item is still an issue we can open a new ticket.