From 5752f68e795d53275c095afd8d332dbae8f2c023 Mon Sep 17 00:00:00 2001 From: "L. R. Couto" <57910428+lrcouto@users.noreply.github.com> Date: Tue, 24 Oct 2023 18:36:06 -0300 Subject: [PATCH] Add addon flags to kedro new (#3081) * Update prompts.yml Signed-off-by: SajidAlamQB <90610031+SajidAlamQB@users.noreply.github.com> * Update starters.py Signed-off-by: SajidAlamQB <90610031+SajidAlamQB@users.noreply.github.com> * add post_gen_project in cookiecutter hooks Signed-off-by: SajidAlamQB <90610031+SajidAlamQB@users.noreply.github.com> * add confirmation message for the options selected Signed-off-by: SajidAlamQB <90610031+SajidAlamQB@users.noreply.github.com> * Update post_gen_project.py Signed-off-by: SajidAlamQB <90610031+SajidAlamQB@users.noreply.github.com> * changes based on review Signed-off-by: SajidAlamQB <90610031+SajidAlamQB@users.noreply.github.com> * Lint Signed-off-by: Ahdra Merali * Remove documentation requirements Signed-off-by: Ahdra Merali * Remove testing requirements Signed-off-by: Ahdra Merali * Remove leftover linting requirements Signed-off-by: Ahdra Merali * Lint Signed-off-by: Ahdra Merali * Add add-on requirements when an addon is selected Signed-off-by: lrcouto * Correct file path Signed-off-by: lrcouto * Update tests with new default template files number Signed-off-by: Ahdra Merali * Update tests with add-ons argument Signed-off-by: Ahdra Merali * Make lint Signed-off-by: Ahdra Merali * Lint Signed-off-by: Ahdra Merali * Make tests use all add-ons by default Signed-off-by: Ahdra Merali * Make unit tests use no add-ons by default Signed-off-by: Ahdra Merali * Add installing project dependencies to e2e tests Signed-off-by: Ahdra Merali * Add linting requirements, organize code Signed-off-by: lrcouto * Refactor test for all add on options Signed-off-by: Ahdra Merali * Add test to check parsing add-ons Signed-off-by: Ahdra Merali * Add scaffolding for add-ons tests Signed-off-by: Ahdra Merali * Change name of test class Signed-off-by: Ahdra Merali * Correct test names Signed-off-by: Ahdra Merali * Correct tests directory Signed-off-by: Ahdra Merali * Clean up success message Signed-off-by: Ahdra Merali * Fix logging option Signed-off-by: Ahdra Merali * Update lint add-on logic Signed-off-by: Ahdra Merali * Ensure add-ons message only shows when add-ons are configurable Signed-off-by: Ahdra Merali * Add requirement checks to tests Signed-off-by: lrcouto * Refactor unit tests Signed-off-by: Ahdra Merali * Add validation to add ons in config file Signed-off-by: Ahdra Merali * Refactor add-ons flow script Signed-off-by: lrcouto * Pass through correct repo name in test Signed-off-by: Ahdra Merali * Clean up and clarify text Signed-off-by: Ahdra Merali * Wrap hook script inside main function Signed-off-by: Ahdra Merali * Revert displayed default Signed-off-by: Ahdra Merali * Add range validation Signed-off-by: Ahdra Merali * Add tests for add-on range validation Signed-off-by: Ahdra Merali * Apply suggestions from code review Co-authored-by: Merel Theisen <49397448+merelcht@users.noreply.github.com> * Update kedro/templates/project/hooks/utils.py Co-authored-by: Merel Theisen <49397448+merelcht@users.noreply.github.com> * Apply suggestions from code review Signed-off-by: Ahdra Merali * Remove traceback from add-on validation - review suggestion Signed-off-by: Ahdra Merali * Output add-on names when selected (via CLI) Signed-off-by: Ahdra Merali * Revert 47d935e and fix tests Signed-off-by: Ahdra Merali * Fix test errors Signed-off-by: Ahdra Merali * Try remove validation Signed-off-by: Ahdra Merali * Add validation back Signed-off-by: Ahdra Merali * Add config file input validation Signed-off-by: Ahdra Merali * Add /site-packages/ to coverage report omit Signed-off-by: Ahdra Merali * Lint Signed-off-by: Ahdra Merali * Remove duplicate error message Signed-off-by: Ahdra Merali * Lint Signed-off-by: Ahdra Merali * Add new flag and help string Signed-off-by: lrcouto * Parse addon list from CLI input Signed-off-by: lrcouto * Overwrite addons from prompts with addons from flag Signed-off-by: lrcouto * Remove addons prompt when addons flag is present Signed-off-by: lrcouto * Use helper function to add addons to config Signed-off-by: lrcouto * Update test to accomodate for new flag Signed-off-by: lrcouto * Add new tests for kedro new Signed-off-by: lrcouto * Fix incorrect docs link Signed-off-by: lrcouto * Fix another incorrect docs link Signed-off-by: lrcouto * Changes to --addons help string Signed-off-by: lrcouto * Fix merge conflics Signed-off-by: lrcouto * Lint Signed-off-by: lrcouto * Fix formatting on --help string Signed-off-by: lrcouto * Change Addons from CLI test to use correct arguments Signed-off-by: lrcouto * Convert addon input strings to number Signed-off-by: lrcouto * Fix tests for addon flags Signed-off-by: lrcouto * Lint Signed-off-by: lrcouto * Clean up project on addons CLI test Signed-off-by: lrcouto * Change error message on invalid addon input Signed-off-by: lrcouto * Make addon CLI test more specific Signed-off-by: lrcouto * Clean up prompt display function Signed-off-by: lrcouto * Handle whitespaces and invalid cases Signed-off-by: lrcouto * Add --addons flag to release notes Signed-off-by: lrcouto --------- Signed-off-by: SajidAlamQB <90610031+SajidAlamQB@users.noreply.github.com> Signed-off-by: Ahdra Merali Signed-off-by: lrcouto Signed-off-by: L. R. Couto <57910428+lrcouto@users.noreply.github.com> Co-authored-by: SajidAlamQB <90610031+SajidAlamQB@users.noreply.github.com> Co-authored-by: Ahdra Merali Co-authored-by: Ahdra Merali <90615669+AhdraMeraliQB@users.noreply.github.com> Co-authored-by: Merel Theisen <49397448+merelcht@users.noreply.github.com> --- RELEASE.md | 1 + kedro/framework/cli/starters.py | 128 ++++++++++++++++++++------- tests/framework/cli/test_starters.py | 79 +++++++++++++++++ tests/tools/test_cli.py | 2 + 4 files changed, 177 insertions(+), 33 deletions(-) diff --git a/RELEASE.md b/RELEASE.md index a1e2a10c7c..8f3915da26 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -22,6 +22,7 @@ ### CLI * Removed deprecated `kedro docs` command. +* Added the `--addons` flag to the `kedro new` command. ### ConfigLoader * `logging` is removed from `ConfigLoader` in favour of the environment variable `KEDRO_LOGGING_CONFIG`. diff --git a/kedro/framework/cli/starters.py b/kedro/framework/cli/starters.py index 66f9471d54..d61812c103 100644 --- a/kedro/framework/cli/starters.py +++ b/kedro/framework/cli/starters.py @@ -11,7 +11,6 @@ import stat import sys import tempfile -import warnings from collections import OrderedDict from itertools import groupby from pathlib import Path @@ -22,7 +21,6 @@ from attrs import define, field import kedro -from kedro import KedroDeprecationWarning from kedro import __version__ as version from kedro.framework.cli.utils import ( CONTEXT_SETTINGS, @@ -37,13 +35,6 @@ TEMPLATE_PATH = KEDRO_PATH / "templates" / "project" _STARTERS_REPO = "git+https://github.com/kedro-org/kedro-starters.git" -_DEPRECATED_STARTERS = [ - "pandas-iris", - "pyspark-iris", - "pyspark", - "standalone-datacatalog", -] - @define(order=True) class KedroStarterSpec: # noqa: too-few-public-methods @@ -107,6 +98,23 @@ class KedroStarterSpec: # noqa: too-few-public-methods "An optional directory inside the repository where the starter resides." ) +# TODO; Insert actual link to the documentation (Visit: kedro.org/{insert-documentation} to find out more about these add-ons.). +ADDON_ARG_HELP = """ +Select which add-ons you'd like to include. By default, none are included.\n + +Add-Ons\n +1) Linting: Provides a basic linting setup with Black and Ruff\n +2) Testing: Provides basic testing setup with pytest\n +3) Custom Logging: Provides more logging options\n +4) Documentation: Basic documentation setup with Sphinx\n +5) Data Structure: Provides a directory structure for storing data\n + +Example usage:\n +kedro new --addons=lint,test,log,docs,data (or any subset of these options)\n +kedro new --addons=all\n +kedro new --addons=none +""" + ADD_ONS_DICT = { "1": "Linting", "2": "Testing", @@ -114,6 +122,7 @@ class KedroStarterSpec: # noqa: too-few-public-methods "4": "Documentation", "5": "Data Structure", } + # noqa: unused-argument def _remove_readonly(func: Callable, path: Path, excinfo: tuple): # pragma: no cover """Remove readonly files on Windows @@ -175,14 +184,10 @@ def _starter_spec_to_dict( """Convert a dictionary of starters spec to a nicely formatted dictionary""" format_dict: dict[str, dict[str, str]] = {} for alias, spec in starter_specs.items(): - if alias in _DEPRECATED_STARTERS: - key = alias + " (deprecated)" - else: - key = alias - format_dict[key] = {} # Each dictionary represent 1 starter - format_dict[key]["template_path"] = spec.template_path + format_dict[alias] = {} # Each dictionary represent 1 starter + format_dict[alias]["template_path"] = spec.template_path if spec.directory: - format_dict[key]["directory"] = spec.directory + format_dict[alias]["directory"] = spec.directory return format_dict @@ -247,21 +252,9 @@ def create_cli(): # pragma: no cover @click.option("--starter", "-s", "starter_alias", help=STARTER_ARG_HELP) @click.option("--checkout", help=CHECKOUT_ARG_HELP) @click.option("--directory", help=DIRECTORY_ARG_HELP) -def new(config_path, starter_alias, checkout, directory, **kwargs): +@click.option("--addons", "-a", "selected_addons", help=ADDON_ARG_HELP) +def new(config_path, starter_alias, selected_addons, checkout, directory, **kwargs): """Create a new kedro project.""" - - if starter_alias in _DEPRECATED_STARTERS: - warnings.warn( - f"The starter '{starter_alias}' has been deprecated and will be archived from Kedro 0.19.0.", - KedroDeprecationWarning, - ) - click.secho( - "From Kedro 0.19.0, the command `kedro new` will come with the option of interactively selecting add-ons " - "for your project such as linting, testing, custom logging, and more. The selected add-ons will add the " - "basic setup for the utilities selected to your projects.", - fg="green", - ) - if checkout and not starter_alias: raise KedroCliError("Cannot use the --checkout flag without a --starter value.") @@ -293,6 +286,10 @@ def new(config_path, starter_alias, checkout, directory, **kwargs): tmpdir = tempfile.mkdtemp() cookiecutter_dir = _get_cookiecutter_dir(template_path, checkout, directory, tmpdir) prompts_required = _get_prompts_required(cookiecutter_dir) + + # Select which prompts will be displayed to the user based on which flags were selected. + prompts_required = _select_prompts_to_display(prompts_required, selected_addons) + # We only need to make cookiecutter_context if interactive prompts are needed. if not config_path: cookiecutter_context = _make_cookiecutter_context_for_prompts(cookiecutter_dir) @@ -318,6 +315,8 @@ def new(config_path, starter_alias, checkout, directory, **kwargs): else: config = _fetch_config_from_user_prompts(prompts_required, cookiecutter_context) + config = _get_addons_from_cli_input(selected_addons, config) + cookiecutter_args = _make_cookiecutter_args(config, checkout, directory) _create_project(template_path, cookiecutter_args) @@ -344,9 +343,6 @@ def list_starters(): sorted_starters_dict = dict( sorted(sorted_starters_dict.items(), key=lambda x: x == "kedro") ) - warnings.warn( - f"The starters {_DEPRECATED_STARTERS} are deprecated and will be archived in Kedro 0.19.0." - ) for origin, starters_spec in sorted_starters_dict.items(): click.secho(f"\nStarters from {origin}\n", fg="yellow") @@ -355,6 +351,72 @@ def list_starters(): ) +def _get_addons_from_cli_input( + selected_addons: str, config: dict[str, str] +) -> dict[str, str]: + """Inserts add-on selection from the CLI input in the project + configuration, if it exists. Replaces add-on strings with the + corresponding prompt number. + + Args: + selected_addons: a string containing the value for the --addons flag, + or None in case the flag wasn't used. + + Returns: + Configuration for starting a new project, with the selected add-ons + from the `--addons` flag. + """ + string_to_number = {"lint": "1", "test": "2", "log": "3", "docs": "4", "data": "5"} + + 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] + config["add_ons"] = ",".join(addons) + + return config + + +def _select_prompts_to_display(prompts_required: dict, selected_addons: str) -> dict: + """Selects which prompts an user will receive when creating a new + Kedro project, based on what information was already made available + through CLI input. + + Args: + prompts_required: a dictionary of all the prompts that will be shown to + the user on project creation. + selected_addons: a string containing the value for the --addons flag, + or None in case the flag wasn't used. + + Returns: + the prompts_required dictionary, with all the redundant information removed. + """ + valid_addons = ["lint", "test", "log", "docs", "data", "all", "none"] + + if selected_addons is not None: + addons = re.sub(r"\s", "", selected_addons).split(",") + for addon in addons: + if addon not in valid_addons: + click.secho( + "Please select from the available add-ons: lint, test, log, docs, data, all, none", + fg="red", + err=True, + ) + sys.exit(1) + if ("none" in addons or "all" in addons) and len(addons) > 1: + click.secho( + "Add-on options 'all' and 'none' cannot be used with other options", + fg="red", + err=True, + ) + sys.exit(1) + del prompts_required["add_ons"] + + return prompts_required + + def _fetch_config_from_file(config_path: str) -> dict[str, str]: """Obtains configuration for a new kedro project non-interactively from a file. diff --git a/tests/framework/cli/test_starters.py b/tests/framework/cli/test_starters.py index 4ac368defa..f0268e7e02 100644 --- a/tests/framework/cli/test_starters.py +++ b/tests/framework/cli/test_starters.py @@ -56,6 +56,24 @@ def _make_cli_prompt_input( return "\n".join([add_ons, project_name, repo_name, python_package]) +def _make_cli_prompt_input_without_addons( + project_name="", repo_name="", python_package="" +): + return "\n".join([project_name, repo_name, python_package]) + + +def _convert_addon_names_to_numbers(selected_addons: str): + string_to_number = {"lint": "1", "test": "2", "log": "3", "docs": "4", "data": "5"} + + 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) + + def _get_expected_files(add_ons: str): add_ons_template_files = { "1": 0, @@ -933,3 +951,64 @@ def test_invalid_add_ons(self, fake_kedro_cli): "Please select valid options for add-ons using comma-separated values, ranges, or 'all/none'.\n" in result.output ) + + +@pytest.mark.usefixtures("chdir_to_tmp") +class TestAddOnsFromCLI: + @pytest.mark.parametrize( + "add_ons", + [ + "lint", + "test", + "log", + "docs", + "data", + "none", + "test,log,docs", + "test,data,lint", + "log,docs,data,test,lint", + "log, docs, data, test, lint", + "log, docs, data, test, lint", + "all", + ], + ) + def test_valid_add_ons(self, fake_kedro_cli, add_ons): + result = CliRunner().invoke( + fake_kedro_cli, + ["new", "--addons", add_ons], + input=_make_cli_prompt_input_without_addons(), + ) + add_ons = _convert_addon_names_to_numbers(selected_addons=add_ons) + _assert_template_ok(result, add_ons=add_ons) + _assert_requirements_ok(result, add_ons=add_ons, repo_name="new-kedro-project") + _clean_up_project(Path("./new-kedro-project")) + + def test_invalid_add_ons(self, fake_kedro_cli): + result = CliRunner().invoke( + fake_kedro_cli, + ["new", "--addons", "bad_input"], + input=_make_cli_prompt_input_without_addons(), + ) + + assert result.exit_code != 0 + assert ( + "Please select from the available add-ons: lint, test, log, docs, data, all, none" + in result.output + ) + + @pytest.mark.parametrize( + "add_ons", + ["lint,all", "test,none", "all,none"], + ) + def test_invalid_add_on_combination(self, fake_kedro_cli, add_ons): + result = CliRunner().invoke( + fake_kedro_cli, + ["new", "--addons", add_ons], + input=_make_cli_prompt_input_without_addons(), + ) + + assert result.exit_code != 0 + assert ( + "Add-on options 'all' and 'none' cannot be used with other options" + in result.output + ) diff --git a/tests/tools/test_cli.py b/tests/tools/test_cli.py index 346d7af6e6..7489126c06 100644 --- a/tests/tools/test_cli.py +++ b/tests/tools/test_cli.py @@ -101,6 +101,8 @@ def test_get_cli_structure_depth(self, mocker, fake_metadata): "-v", "--config", "-c", + "--addons", + "-a", "--starter", "-s", "--checkout",