From 93fded80875251c58921558ee409339b3eb30988 Mon Sep 17 00:00:00 2001 From: Sajid Alam Date: Thu, 14 Dec 2023 13:16:20 +0000 Subject: [PATCH 01/19] unique tool entires and QoL for pyproject.toml Signed-off-by: Sajid Alam Signed-off-by: Ahdra Merali --- kedro/framework/cli/starters.py | 4 ++++ .../project/{{ cookiecutter.repo_name }}/pyproject.toml | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/kedro/framework/cli/starters.py b/kedro/framework/cli/starters.py index cd78155372..49554b85a3 100644 --- a/kedro/framework/cli/starters.py +++ b/kedro/framework/cli/starters.py @@ -841,6 +841,10 @@ def _validate_range(start, end): else: selected.append(choice.strip()) + # Ensure unique entries only + seen = set() + selected = [x for x in selected if not (x in seen or seen.add(x))] + return selected diff --git a/kedro/templates/project/{{ cookiecutter.repo_name }}/pyproject.toml b/kedro/templates/project/{{ cookiecutter.repo_name }}/pyproject.toml index 07592a3276..8bcb65a9e5 100644 --- a/kedro/templates/project/{{ cookiecutter.repo_name }}/pyproject.toml +++ b/kedro/templates/project/{{ cookiecutter.repo_name }}/pyproject.toml @@ -37,7 +37,8 @@ namespaces = false package_name = "{{ cookiecutter.python_package }}" project_name = "{{ cookiecutter.project_name }}" kedro_init_version = "{{ cookiecutter.kedro_version }}" -tools = "{{ cookiecutter.tools | default('') | string | replace('\"', '\\\"') }}" +tools = {{ cookiecutter.tools | default('') | string | replace('\"', '\\\"') }} +example_pipeline = "{{ cookiecutter.example_pipeline }}" [tool.pytest.ini_options] addopts = """ From 39c1951713f5faf9ad6afdb671719d06602514bb Mon Sep 17 00:00:00 2001 From: Sajid Alam Date: Thu, 14 Dec 2023 13:32:43 +0000 Subject: [PATCH 02/19] revert unique entries change Signed-off-by: Sajid Alam Signed-off-by: Ahdra Merali --- kedro/framework/cli/starters.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/kedro/framework/cli/starters.py b/kedro/framework/cli/starters.py index 49554b85a3..cd78155372 100644 --- a/kedro/framework/cli/starters.py +++ b/kedro/framework/cli/starters.py @@ -841,10 +841,6 @@ def _validate_range(start, end): else: selected.append(choice.strip()) - # Ensure unique entries only - seen = set() - selected = [x for x in selected if not (x in seen or seen.add(x))] - return selected From 6abe2981383918f252ef6125486dc81299f61897 Mon Sep 17 00:00:00 2001 From: Ahdra Merali <90615669+AhdraMeraliQB@users.noreply.github.com> Date: Fri, 15 Dec 2023 11:36:35 +0000 Subject: [PATCH 03/19] Update cli.py template (#3428) Signed-off-by: Ahdra Merali --- docs/source/development/commands_reference.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/source/development/commands_reference.md b/docs/source/development/commands_reference.md index b3d1911d3e..ca478e863a 100644 --- a/docs/source/development/commands_reference.md +++ b/docs/source/development/commands_reference.md @@ -162,9 +162,8 @@ from kedro.framework.cli.project import ( from kedro.framework.cli.utils import ( CONTEXT_SETTINGS, _config_file_callback, - _get_values_as_tuple, - _reformat_load_versions, _split_params, + _split_load_versions, env_option, split_string, split_node_names, @@ -204,7 +203,7 @@ def cli(): type=str, multiple=True, help=LOAD_VERSION_HELP, - callback=_reformat_load_versions, + callback=_split_load_versions, ) @click.option("--pipeline", "-p", type=str, default=None, help=PIPELINE_ARG_HELP) @click.option( From b643b23cbf1c82178cbdbfe64a51cbe651b357af Mon Sep 17 00:00:00 2001 From: Merel Theisen <49397448+merelcht@users.noreply.github.com> Date: Fri, 15 Dec 2023 17:09:27 +0000 Subject: [PATCH 04/19] Add logging about not using async for Sequential and Parallel runners (#3424) Signed-off-by: Merel Theisen Signed-off-by: Ahdra Merali --- RELEASE.md | 3 +-- kedro/runner/parallel_runner.py | 5 +++++ kedro/runner/sequential_runner.py | 5 +++++ tests/runner/test_parallel_runner.py | 5 +++++ tests/runner/test_sequential_runner.py | 5 +++++ tests/runner/test_thread_runner.py | 5 +++++ 6 files changed, 26 insertions(+), 2 deletions(-) diff --git a/RELEASE.md b/RELEASE.md index 401d6efc3a..f5ddf807c2 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -3,8 +3,7 @@ ## Major features and improvements ## Bug fixes and other changes - -## Breaking changes to the API +* Added logging about not using async mode in `SequentiallRunner` and `ParallelRunner`. ## Documentation changes diff --git a/kedro/runner/parallel_runner.py b/kedro/runner/parallel_runner.py index 309eef0708..9d4081f121 100644 --- a/kedro/runner/parallel_runner.py +++ b/kedro/runner/parallel_runner.py @@ -256,6 +256,11 @@ def _run( # noqa: too-many-locals,useless-suppression """ # noqa: import-outside-toplevel,cyclic-import + if not self._is_async: + self._logger.info( + "Using synchronous mode for loading and saving data. Use the --async flag " + "for potential performance gains. https://docs.kedro.org/en/stable/nodes_and_pipelines/run_a_pipeline.html#load-and-save-asynchronously" + ) nodes = pipeline.nodes self._validate_catalog(catalog, pipeline) diff --git a/kedro/runner/sequential_runner.py b/kedro/runner/sequential_runner.py index a93f8642a0..3270df5122 100644 --- a/kedro/runner/sequential_runner.py +++ b/kedro/runner/sequential_runner.py @@ -60,6 +60,11 @@ def _run( Raises: Exception: in case of any downstream node failure. """ + if not self._is_async: + self._logger.info( + "Using synchronous mode for loading and saving data. Use the --async flag " + "for potential performance gains. https://docs.kedro.org/en/stable/nodes_and_pipelines/run_a_pipeline.html#load-and-save-asynchronously" + ) nodes = pipeline.nodes done_nodes = set() diff --git a/tests/runner/test_parallel_runner.py b/tests/runner/test_parallel_runner.py index e931bdbf7a..fb967b2cb8 100644 --- a/tests/runner/test_parallel_runner.py +++ b/tests/runner/test_parallel_runner.py @@ -73,6 +73,11 @@ def test_memory_dataset_input(self, is_async, fan_out_fan_in): assert len(result["Z"]) == 3 assert result["Z"] == ("42", "42", "42") + def test_log_not_using_async(self, fan_out_fan_in, catalog, caplog): + catalog.add_feed_dict({"A": 42}) + ParallelRunner().run(fan_out_fan_in, catalog) + assert "Using synchronous mode for loading and saving data." in caplog.text + class TestMaxWorkers: @pytest.mark.parametrize("is_async", [False, True]) diff --git a/tests/runner/test_sequential_runner.py b/tests/runner/test_sequential_runner.py index db6b07a50e..3ee297c0a7 100644 --- a/tests/runner/test_sequential_runner.py +++ b/tests/runner/test_sequential_runner.py @@ -29,6 +29,11 @@ def test_run_without_plugin_manager(self, fan_out_fan_in, catalog): assert "Z" in result assert result["Z"] == (42, 42, 42) + def test_log_not_using_async(self, fan_out_fan_in, catalog, caplog): + catalog.add_feed_dict({"A": 42}) + SequentialRunner().run(fan_out_fan_in, catalog) + assert "Using synchronous mode for loading and saving data." in caplog.text + @pytest.mark.parametrize("is_async", [False, True]) class TestSeqentialRunnerBranchlessPipeline: diff --git a/tests/runner/test_thread_runner.py b/tests/runner/test_thread_runner.py index 5d2efefe8c..b570f35cbf 100644 --- a/tests/runner/test_thread_runner.py +++ b/tests/runner/test_thread_runner.py @@ -34,6 +34,11 @@ def test_memory_dataset_input(self, fan_out_fan_in): assert "Z" in result assert result["Z"] == ("42", "42", "42") + def test_does_not_log_not_using_async(self, fan_out_fan_in, catalog, caplog): + catalog.add_feed_dict({"A": 42}) + ThreadRunner().run(fan_out_fan_in, catalog) + assert "Using synchronous mode for loading and saving data." not in caplog.text + class TestMaxWorkers: @pytest.mark.parametrize( From f8c2a8e7971970f98cb20128b2fad87341dfb51c Mon Sep 17 00:00:00 2001 From: Sajid Alam <90610031+SajidAlamQB@users.noreply.github.com> Date: Mon, 18 Dec 2023 12:25:21 +0000 Subject: [PATCH 05/19] Don't include requirements only needed for example pipeline (#3425) * remove example pipeline requirements Signed-off-by: Sajid Alam * lint Signed-off-by: Sajid Alam * simplify amending kedro[...] lines Signed-off-by: Sajid Alam * keep the version for datasets Signed-off-by: Sajid Alam * lint Signed-off-by: Sajid Alam --------- Signed-off-by: Sajid Alam Signed-off-by: Sajid Alam <90610031+SajidAlamQB@users.noreply.github.com> Signed-off-by: Ahdra Merali --- RELEASE.md | 3 +++ kedro/templates/project/hooks/utils.py | 37 +++++++++++++++++++++++--- 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/RELEASE.md b/RELEASE.md index f5ddf807c2..bbcbd98836 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -3,6 +3,9 @@ ## Major features and improvements ## Bug fixes and other changes +* Removed example pipeline requirements when examples are not selected in `tools`. + +## Breaking changes to the API * Added logging about not using async mode in `SequentiallRunner` and `ParallelRunner`. ## Documentation changes diff --git a/kedro/templates/project/hooks/utils.py b/kedro/templates/project/hooks/utils.py index 1f2deb799e..4055c147a5 100644 --- a/kedro/templates/project/hooks/utils.py +++ b/kedro/templates/project/hooks/utils.py @@ -15,6 +15,9 @@ # Configuration key for documentation dependencies docs_pyproject_requirements = ["project.optional-dependencies"] # For pyproject.toml +# Requirements for example pipelines +example_pipeline_requirements = "seaborn~=0.12.1\nscikit-learn~=1.0\n" + # Helper Functions def _remove_from_file(file_path: Path, content_to_remove: str) -> None: @@ -141,13 +144,35 @@ def _remove_pyspark_viz_starter_files(is_viz: bool, python_package_name: str) -> _remove_file(test_pipeline_path) -def setup_template_tools(selected_tools_list: str, requirements_file_path: str, pyproject_file_path: str, python_package_name: str, example_pipeline: str) -> None: - """Setup the templates according to the choice of tools. +def _remove_extras_from_kedro_datasets(file_path: Path) -> None: + """Remove all extras from kedro-datasets in the requirements file, while keeping the version. + + Args: + file_path (Path): The path of the requirements file. + """ + with open(file_path, 'r') as file: + lines = file.readlines() + + for i, line in enumerate(lines): + if 'kedro-datasets[' in line: + # Split the line at '[', and keep the part before it + package = line.split('[', 1)[0] + # Extract version + version = line.split(']')[-1] + lines[i] = package + version + + with open(file_path, 'w') as file: + file.writelines(lines) + + +def setup_template_tools(selected_tools_list: str, requirements_file_path: Path, pyproject_file_path: Path, + python_package_name: str, example_pipeline: str) -> None: + """Set up the templates according to the choice of tools. Args: selected_tools_list (str): A string contains the selected tools. - requirements_file_path (str): The path of the `requiremenets.txt` in the template. - pyproject_file_path (str): The path of the `pyproject.toml` in the template + requirements_file_path (Path): The path of the `requiremenets.txt` in the template. + pyproject_file_path (Path): The path of the `pyproject.toml` in the template python_package_name (str): The name of the python package. example_pipeline (str): 'True' if example pipeline was selected """ @@ -172,6 +197,10 @@ def setup_template_tools(selected_tools_list: str, requirements_file_path: str, if ("PySpark" in selected_tools_list or "Kedro Viz" in selected_tools_list) and example_pipeline != "True": _remove_pyspark_viz_starter_files("Kedro Viz" in selected_tools_list, python_package_name) + # Remove requirements used by example pipelines + _remove_from_file(requirements_file_path, example_pipeline_requirements) + _remove_extras_from_kedro_datasets(requirements_file_path) + def sort_requirements(requirements_file_path: Path) -> None: """Sort the requirements.txt file alphabetically and write it back to the file. From 848b4dc5551daa1df655607edf08944060b102c8 Mon Sep 17 00:00:00 2001 From: Deepyaman Datta Date: Mon, 18 Dec 2023 10:14:05 -0700 Subject: [PATCH 06/19] Sort `requirements.txt` based on package name only (#3436) * Sort `requirements.txt` based on package name only Signed-off-by: Deepyaman Datta * Remove now-unused custom `requirements.txt` sorter Signed-off-by: Deepyaman Datta * Ruff format kedro/templates/project/hooks/utils.py Signed-off-by: Deepyaman Datta * Pass the right argument to `fix_requirements` call Signed-off-by: Deepyaman Datta * Add `sort_requirements` until starters are updated Signed-off-by: Deepyaman Datta * Wrap lib call in existing method for compatibility Signed-off-by: Deepyaman Datta --------- Signed-off-by: Deepyaman Datta Signed-off-by: Ahdra Merali --- .../project/hooks/post_gen_project.py | 17 +++-- kedro/templates/project/hooks/utils.py | 71 +++++++++++-------- pyproject.toml | 1 + 3 files changed, 52 insertions(+), 37 deletions(-) diff --git a/kedro/templates/project/hooks/post_gen_project.py b/kedro/templates/project/hooks/post_gen_project.py index 7b607fde68..2e17b3403a 100644 --- a/kedro/templates/project/hooks/post_gen_project.py +++ b/kedro/templates/project/hooks/post_gen_project.py @@ -1,27 +1,30 @@ from pathlib import Path -from kedro.templates.project.hooks.utils import ( - - setup_template_tools, - sort_requirements, -) +from kedro.templates.project.hooks.utils import setup_template_tools, sort_requirements def main(): current_dir = Path.cwd() requirements_file_path = current_dir / "requirements.txt" pyproject_file_path = current_dir / "pyproject.toml" - python_package_name = '{{ cookiecutter.python_package }}' + python_package_name = "{{ cookiecutter.python_package }}" # Get the selected tools from cookiecutter selected_tools = "{{ cookiecutter.tools }}" example_pipeline = "{{ cookiecutter.example_pipeline }}" # Handle template directories and requirements according to selected tools - setup_template_tools(selected_tools, requirements_file_path, pyproject_file_path, python_package_name, example_pipeline) + setup_template_tools( + selected_tools, + requirements_file_path, + pyproject_file_path, + python_package_name, + example_pipeline, + ) # Sort requirements.txt file in alphabetical order sort_requirements(requirements_file_path) + if __name__ == "__main__": main() diff --git a/kedro/templates/project/hooks/utils.py b/kedro/templates/project/hooks/utils.py index 4055c147a5..7c3b3a1612 100644 --- a/kedro/templates/project/hooks/utils.py +++ b/kedro/templates/project/hooks/utils.py @@ -2,6 +2,8 @@ import shutil import toml +from pre_commit_hooks.requirements_txt_fixer import fix_requirements + current_dir = Path.cwd() # Requirements for linting tools @@ -9,8 +11,13 @@ lint_pyproject_requirements = ["tool.ruff"] # For pyproject.toml # Requirements and configurations for testing tools and coverage reporting -test_requirements = "pytest-cov~=3.0\npytest-mock>=1.7.1, <2.0\npytest~=7.2" # For requirements.txt -test_pyproject_requirements = ["tool.pytest.ini_options", "tool.coverage.report"] # For pyproject.toml +test_requirements = ( # For requirements.txt + "pytest-cov~=3.0\npytest-mock>=1.7.1, <2.0\npytest~=7.2" +) +test_pyproject_requirements = [ # For pyproject.toml + "tool.pytest.ini_options", + "tool.coverage.report", +] # Configuration key for documentation dependencies docs_pyproject_requirements = ["project.optional-dependencies"] # For pyproject.toml @@ -27,16 +34,16 @@ def _remove_from_file(file_path: Path, content_to_remove: str) -> None: file_path (Path): The path of the file from which to remove content. content_to_remove (str): The content to be removed from the file. """ - with open(file_path, 'r') as file: + with open(file_path, "r") as file: lines = file.readlines() # Split the content to remove into lines and remove trailing whitespaces/newlines - content_to_remove_lines = [line.strip() for line in content_to_remove.split('\n')] + content_to_remove_lines = [line.strip() for line in content_to_remove.split("\n")] # Keep lines that are not in content_to_remove lines = [line for line in lines if line.strip() not in content_to_remove_lines] - with open(file_path, 'w') as file: + with open(file_path, "w") as file: file.writelines(lines) @@ -47,7 +54,7 @@ def _remove_nested_section(data: dict, nested_key: str) -> None: data (dict): The dictionary from which to remove the section. nested_key (str): The dotted path key representing the nested section to remove. """ - keys = nested_key.split('.') + keys = nested_key.split(".") current_data = data # Look for Parent section for key in keys[:-1]: # Iterate over all but last element @@ -60,7 +67,7 @@ def _remove_nested_section(data: dict, nested_key: str) -> None: current_data.pop(keys[-1], None) # Remove last element otherwise return None for key in reversed(keys[:-1]): parent_section = data - for key_part in keys[:keys.index(key)]: + for key_part in keys[: keys.index(key)]: parent_section = parent_section[key_part] if not current_data: # If the section is empty, remove it parent_section.pop(key, None) @@ -77,14 +84,14 @@ def _remove_from_toml(file_path: Path, sections_to_remove: list) -> None: sections_to_remove (list): A list of section keys to remove from the TOML file. """ # Load the TOML file - with open(file_path, 'r') as file: + with open(file_path, "r") as file: data = toml.load(file) # Remove the specified sections for section in sections_to_remove: _remove_nested_section(data, section) - with open(file_path, 'w') as file: + with open(file_path, "w") as file: toml.dump(data, file) @@ -124,7 +131,7 @@ def _remove_pyspark_viz_starter_files(is_viz: bool, python_package_name: str) -> # Empty the contents of conf/base/catalog.yml catalog_yml_path = current_dir / "conf/base/catalog.yml" if catalog_yml_path.exists(): - catalog_yml_path.write_text('') + catalog_yml_path.write_text("") # Remove parameter files from conf/base conf_base_path = current_dir / "conf/base/" parameter_file_patterns = ["parameters_*.yml", "parameters/*.yml"] @@ -133,7 +140,9 @@ def _remove_pyspark_viz_starter_files(is_viz: bool, python_package_name: str) -> _remove_file(param_file) # Remove the pipelines subdirectories, if Viz - also "reporting" folder - pipelines_to_remove = ["data_science", "data_processing"] + (["reporting"] if is_viz else []) + pipelines_to_remove = ["data_science", "data_processing"] + ( + ["reporting"] if is_viz else [] + ) pipelines_path = current_dir / f"src/{python_package_name}/pipelines/" for pipeline_subdir in pipelines_to_remove: @@ -150,23 +159,28 @@ def _remove_extras_from_kedro_datasets(file_path: Path) -> None: Args: file_path (Path): The path of the requirements file. """ - with open(file_path, 'r') as file: + with open(file_path, "r") as file: lines = file.readlines() for i, line in enumerate(lines): - if 'kedro-datasets[' in line: + if "kedro-datasets[" in line: # Split the line at '[', and keep the part before it - package = line.split('[', 1)[0] + package = line.split("[", 1)[0] # Extract version - version = line.split(']')[-1] + version = line.split("]")[-1] lines[i] = package + version - with open(file_path, 'w') as file: + with open(file_path, "w") as file: file.writelines(lines) -def setup_template_tools(selected_tools_list: str, requirements_file_path: Path, pyproject_file_path: Path, - python_package_name: str, example_pipeline: str) -> None: +def setup_template_tools( + selected_tools_list: str, + requirements_file_path: Path, + pyproject_file_path: Path, + python_package_name: str, + example_pipeline: str, +) -> None: """Set up the templates according to the choice of tools. Args: @@ -195,25 +209,22 @@ def setup_template_tools(selected_tools_list: str, requirements_file_path: Path, if "Data Structure" not in selected_tools_list and example_pipeline != "True": _remove_dir(current_dir / "data") - if ("PySpark" in selected_tools_list or "Kedro Viz" in selected_tools_list) and example_pipeline != "True": - _remove_pyspark_viz_starter_files("Kedro Viz" in selected_tools_list, python_package_name) + if ( + "PySpark" in selected_tools_list or "Kedro Viz" in selected_tools_list + ) and example_pipeline != "True": + _remove_pyspark_viz_starter_files( + "Kedro Viz" in selected_tools_list, python_package_name + ) # Remove requirements used by example pipelines _remove_from_file(requirements_file_path, example_pipeline_requirements) _remove_extras_from_kedro_datasets(requirements_file_path) def sort_requirements(requirements_file_path: Path) -> None: - """Sort the requirements.txt file alphabetically and write it back to the file. + """Sort entries in `requirements.txt`, writing back changes, if any. Args: requirements_file_path (Path): The path to the `requirements.txt` file. """ - with open(requirements_file_path, 'r') as requirements: - lines = requirements.readlines() - - lines = [line.strip() for line in lines] - lines.sort() - sorted_content = '\n'.join(lines) - - with open(requirements_file_path, 'w') as requirements: - requirements.write(sorted_content) + with open(requirements_file_path, "rb+") as file_obj: + fix_requirements(file_obj) diff --git a/pyproject.toml b/pyproject.toml index 4a7079a89b..9cdae213c5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -28,6 +28,7 @@ dependencies = [ "omegaconf>=2.1.1", "parse>=1.19.0", "pluggy>=1.0", + "pre-commit-hooks", "PyYAML>=4.2,<7.0", "rich>=12.0,<14.0", "rope>=0.21,<2.0", # subject to LGPLv3 license From bf9cf825ff0aa6345455dc0416a4e8ea4359324f Mon Sep 17 00:00:00 2001 From: Ahdra Merali Date: Tue, 19 Dec 2023 16:14:21 +0000 Subject: [PATCH 07/19] Pass tools through as list Signed-off-by: Ahdra Merali --- kedro/framework/cli/starters.py | 24 +++++++++---------- .../pyproject.toml | 2 +- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/kedro/framework/cli/starters.py b/kedro/framework/cli/starters.py index cd78155372..3812ef7cbf 100644 --- a/kedro/framework/cli/starters.py +++ b/kedro/framework/cli/starters.py @@ -463,7 +463,8 @@ def _get_extra_context( # noqa: PLR0913 NUMBER_TO_TOOLS_NAME[tool] for tool in _parse_tools_input(tools) # type: ignore ] - extra_context["tools"] = str(extra_context["tools"]) + else: + extra_context["tools"] = [] # type: ignore extra_context["example_pipeline"] = ( _parse_yes_no_to_bool( @@ -879,17 +880,16 @@ def _create_project(template_path: str, cookiecutter_args: dict[str, Any]): ) # we can use starters without tools: - if tools is not None: - if tools == "[]": # TODO: This should be a list - click.secho( - "You have selected no project tools", - fg="green", - ) - else: - click.secho( - f"You have selected the following project tools: {tools}", - fg="green", - ) + if tools == []: + click.secho( + "You have selected no project tools", + fg="green", + ) + else: + click.secho( + f"You have selected the following project tools: {tools}", + fg="green", + ) if example_pipeline is not None: if example_pipeline: diff --git a/kedro/templates/project/{{ cookiecutter.repo_name }}/pyproject.toml b/kedro/templates/project/{{ cookiecutter.repo_name }}/pyproject.toml index 8bcb65a9e5..0028c35f0a 100644 --- a/kedro/templates/project/{{ cookiecutter.repo_name }}/pyproject.toml +++ b/kedro/templates/project/{{ cookiecutter.repo_name }}/pyproject.toml @@ -37,7 +37,7 @@ namespaces = false package_name = "{{ cookiecutter.python_package }}" project_name = "{{ cookiecutter.project_name }}" kedro_init_version = "{{ cookiecutter.kedro_version }}" -tools = {{ cookiecutter.tools | default('') | string | replace('\"', '\\\"') }} +tools = {{ cookiecutter.tools | default([]) | list | replace('\"', '\\\"') }} example_pipeline = "{{ cookiecutter.example_pipeline }}" [tool.pytest.ini_options] From c96c5a094f441d466b8def89cda7e7bf96550b94 Mon Sep 17 00:00:00 2001 From: Ahdra Merali Date: Tue, 19 Dec 2023 18:05:15 +0000 Subject: [PATCH 08/19] Revert "Pass tools through as list" This reverts commit f4a4a150f160efb7301e270a4d62122ffc183622. Signed-off-by: Ahdra Merali --- kedro/framework/cli/starters.py | 24 +++++++++---------- .../pyproject.toml | 2 +- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/kedro/framework/cli/starters.py b/kedro/framework/cli/starters.py index 3812ef7cbf..cd78155372 100644 --- a/kedro/framework/cli/starters.py +++ b/kedro/framework/cli/starters.py @@ -463,8 +463,7 @@ def _get_extra_context( # noqa: PLR0913 NUMBER_TO_TOOLS_NAME[tool] for tool in _parse_tools_input(tools) # type: ignore ] - else: - extra_context["tools"] = [] # type: ignore + extra_context["tools"] = str(extra_context["tools"]) extra_context["example_pipeline"] = ( _parse_yes_no_to_bool( @@ -880,16 +879,17 @@ def _create_project(template_path: str, cookiecutter_args: dict[str, Any]): ) # we can use starters without tools: - if tools == []: - click.secho( - "You have selected no project tools", - fg="green", - ) - else: - click.secho( - f"You have selected the following project tools: {tools}", - fg="green", - ) + if tools is not None: + if tools == "[]": # TODO: This should be a list + click.secho( + "You have selected no project tools", + fg="green", + ) + else: + click.secho( + f"You have selected the following project tools: {tools}", + fg="green", + ) if example_pipeline is not None: if example_pipeline: diff --git a/kedro/templates/project/{{ cookiecutter.repo_name }}/pyproject.toml b/kedro/templates/project/{{ cookiecutter.repo_name }}/pyproject.toml index 0028c35f0a..8bcb65a9e5 100644 --- a/kedro/templates/project/{{ cookiecutter.repo_name }}/pyproject.toml +++ b/kedro/templates/project/{{ cookiecutter.repo_name }}/pyproject.toml @@ -37,7 +37,7 @@ namespaces = false package_name = "{{ cookiecutter.python_package }}" project_name = "{{ cookiecutter.project_name }}" kedro_init_version = "{{ cookiecutter.kedro_version }}" -tools = {{ cookiecutter.tools | default([]) | list | replace('\"', '\\\"') }} +tools = {{ cookiecutter.tools | default('') | string | replace('\"', '\\\"') }} example_pipeline = "{{ cookiecutter.example_pipeline }}" [tool.pytest.ini_options] From 8d167264eb77b1cc3c97ca1a353d03aff5f93db2 Mon Sep 17 00:00:00 2001 From: Ahdra Merali Date: Tue, 19 Dec 2023 18:11:06 +0000 Subject: [PATCH 09/19] Remove duplicates Signed-off-by: Ahdra Merali --- kedro/framework/cli/starters.py | 4 ++++ tests/framework/cli/test_starters.py | 5 +++++ 2 files changed, 9 insertions(+) diff --git a/kedro/framework/cli/starters.py b/kedro/framework/cli/starters.py index cd78155372..91d1b6842e 100644 --- a/kedro/framework/cli/starters.py +++ b/kedro/framework/cli/starters.py @@ -501,6 +501,10 @@ def _convert_tool_names_to_numbers(selected_tools: str | None) -> str | None: tool_short_name = tool.strip() if tool_short_name in TOOLS_SHORTNAME_TO_NUMBER: tools.append(TOOLS_SHORTNAME_TO_NUMBER[tool_short_name]) + + # Remove duplicates if any + tools = sorted(list(set(tools))) + return ",".join(tools) diff --git a/tests/framework/cli/test_starters.py b/tests/framework/cli/test_starters.py index 13f621857f..2f5e7a535e 100644 --- a/tests/framework/cli/test_starters.py +++ b/tests/framework/cli/test_starters.py @@ -1461,3 +1461,8 @@ def test_convert_tool_names_to_numbers_with_invalid_tools(self): selected_tools = "invalid_tool1,invalid_tool2" result = _convert_tool_names_to_numbers(selected_tools) assert result == "" + + def test_convert_tool_names_to_numbers_with_duplicates(self): + selected_tools = "lint,test,tests" + result = _convert_tool_names_to_numbers(selected_tools) + assert result == "1,2" From 9a2d5d57964f748b440a0d8598456a62526f9e40 Mon Sep 17 00:00:00 2001 From: Ahdra Merali Date: Tue, 19 Dec 2023 19:11:12 +0000 Subject: [PATCH 10/19] Fix for no add-ons selected Signed-off-by: Ahdra Merali --- kedro/framework/cli/starters.py | 6 ++++-- tests/framework/cli/test_starters.py | 6 +++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/kedro/framework/cli/starters.py b/kedro/framework/cli/starters.py index 91d1b6842e..1e6f9831f0 100644 --- a/kedro/framework/cli/starters.py +++ b/kedro/framework/cli/starters.py @@ -464,6 +464,8 @@ def _get_extra_context( # noqa: PLR0913 for tool in _parse_tools_input(tools) # type: ignore ] extra_context["tools"] = str(extra_context["tools"]) + else: + extra_context["tools"] = str(["None"]) extra_context["example_pipeline"] = ( _parse_yes_no_to_bool( @@ -492,7 +494,7 @@ def _convert_tool_names_to_numbers(selected_tools: str | None) -> str | None: if selected_tools is None: return None if selected_tools.lower() == "none": - return "" + return None if selected_tools.lower() == "all": return ",".join(NUMBER_TO_TOOLS_NAME.keys()) @@ -808,7 +810,7 @@ def _validate_selection(tools: list[str]): sys.exit(1) -def _parse_tools_input(tools_str: str): +def _parse_tools_input(tools_str: str | None): """Parse the tools input string. Args: diff --git a/tests/framework/cli/test_starters.py b/tests/framework/cli/test_starters.py index 2f5e7a535e..d8dac8e121 100644 --- a/tests/framework/cli/test_starters.py +++ b/tests/framework/cli/test_starters.py @@ -1290,7 +1290,11 @@ def test_valid_tools_flag(self, fake_kedro_cli, tools, example_pipeline): ["new", "--tools", tools, "--example", example_pipeline], input=_make_cli_prompt_input_without_tools(), ) + tools = _convert_tool_names_to_numbers(selected_tools=tools) + if not tools: + tools = "" + _assert_template_ok(result, tools=tools, example_pipeline=example_pipeline) _assert_requirements_ok(result, tools=tools, repo_name="new-kedro-project") _clean_up_project(Path("./new-kedro-project")) @@ -1436,7 +1440,7 @@ def test_convert_tool_names_to_numbers_with_empty_string(self): def test_convert_tool_names_to_numbers_with_none_string(self): selected_tools = "none" result = _convert_tool_names_to_numbers(selected_tools) - assert result == "" + assert result == None def test_convert_tool_names_to_numbers_with_all_string(self): result = _convert_tool_names_to_numbers("all") From 8e6f6b87d0f74d406033e47a2fdf344d1c33f447 Mon Sep 17 00:00:00 2001 From: Ahdra Merali Date: Wed, 20 Dec 2023 10:50:17 +0000 Subject: [PATCH 11/19] Lint Signed-off-by: Ahdra Merali --- kedro/framework/cli/starters.py | 2 +- tests/framework/cli/test_starters.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/kedro/framework/cli/starters.py b/kedro/framework/cli/starters.py index a4434e9c66..2b046e35ea 100644 --- a/kedro/framework/cli/starters.py +++ b/kedro/framework/cli/starters.py @@ -768,7 +768,7 @@ def _validate_selection(tools: list[str]): sys.exit(1) -def _parse_tools_input(tools_str: str | None): +def _parse_tools_input(tools_str: str): """Parse the tools input string. Args: diff --git a/tests/framework/cli/test_starters.py b/tests/framework/cli/test_starters.py index 02a355b7a6..a826157955 100644 --- a/tests/framework/cli/test_starters.py +++ b/tests/framework/cli/test_starters.py @@ -1440,7 +1440,7 @@ def test_convert_tool_names_to_numbers_with_empty_string(self): def test_convert_tool_names_to_numbers_with_none_string(self): selected_tools = "none" result = _convert_tool_names_to_numbers(selected_tools) - assert result == None + assert result is None def test_convert_tool_names_to_numbers_with_all_string(self): result = _convert_tool_names_to_numbers("all") From f31ae5910bb30a98c20c8337d89d0dd5d0dac14d Mon Sep 17 00:00:00 2001 From: Ahdra Merali Date: Wed, 20 Dec 2023 11:20:54 +0000 Subject: [PATCH 12/19] Add example as recognised key Signed-off-by: Ahdra Merali --- kedro/framework/startup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kedro/framework/startup.py b/kedro/framework/startup.py index 3dcbed9e91..a13478054d 100644 --- a/kedro/framework/startup.py +++ b/kedro/framework/startup.py @@ -112,7 +112,7 @@ def _get_project_metadata(project_path: Union[str, Path]) -> ProjectMetadata: try: return ProjectMetadata(**metadata_dict) except TypeError as exc: - expected_keys = mandatory_keys + ["source_dir", "tools"] + expected_keys = mandatory_keys + ["source_dir", "tools", "example"] raise RuntimeError( f"Found unexpected keys in '{_PYPROJECT}'. Make sure " f"it only contains the following keys: {expected_keys}." From 36866fdab54cd7d091f2571b8b6612ff448deccb Mon Sep 17 00:00:00 2001 From: Ahdra Merali Date: Wed, 20 Dec 2023 11:29:24 +0000 Subject: [PATCH 13/19] Add example as recognised key pt2 Signed-off-by: Ahdra Merali --- tests/framework/test_startup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/framework/test_startup.py b/tests/framework/test_startup.py index b67e0e2d1a..0fb0ac28a9 100644 --- a/tests/framework/test_startup.py +++ b/tests/framework/test_startup.py @@ -141,7 +141,7 @@ def test_toml_file_with_extra_keys(self, mocker): pattern = ( "Found unexpected keys in 'pyproject.toml'. Make sure it " "only contains the following keys: ['package_name', " - "'project_name', 'kedro_init_version', 'source_dir', 'tools']." + "'project_name', 'kedro_init_version', 'source_dir', 'tools', 'example']." ) with pytest.raises(RuntimeError, match=re.escape(pattern)): From 47ef1f2db547d35e6567d3183f4370e1351ea7c8 Mon Sep 17 00:00:00 2001 From: Ahdra Merali Date: Wed, 20 Dec 2023 11:48:32 +0000 Subject: [PATCH 14/19] Add example as recognised key pt3 Signed-off-by: Ahdra Merali --- kedro/framework/startup.py | 2 +- tests/framework/test_startup.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/kedro/framework/startup.py b/kedro/framework/startup.py index a13478054d..37aa27819a 100644 --- a/kedro/framework/startup.py +++ b/kedro/framework/startup.py @@ -112,7 +112,7 @@ def _get_project_metadata(project_path: Union[str, Path]) -> ProjectMetadata: try: return ProjectMetadata(**metadata_dict) except TypeError as exc: - expected_keys = mandatory_keys + ["source_dir", "tools", "example"] + expected_keys = mandatory_keys + ["source_dir", "tools", "example_pipeline"] raise RuntimeError( f"Found unexpected keys in '{_PYPROJECT}'. Make sure " f"it only contains the following keys: {expected_keys}." diff --git a/tests/framework/test_startup.py b/tests/framework/test_startup.py index 0fb0ac28a9..d8de98315a 100644 --- a/tests/framework/test_startup.py +++ b/tests/framework/test_startup.py @@ -141,7 +141,7 @@ def test_toml_file_with_extra_keys(self, mocker): pattern = ( "Found unexpected keys in 'pyproject.toml'. Make sure it " "only contains the following keys: ['package_name', " - "'project_name', 'kedro_init_version', 'source_dir', 'tools', 'example']." + "'project_name', 'kedro_init_version', 'source_dir', 'tools', 'example_pipeline']." ) with pytest.raises(RuntimeError, match=re.escape(pattern)): From 6e29a8c4825a9d67229e58fba271d60c362094be Mon Sep 17 00:00:00 2001 From: Ahdra Merali Date: Wed, 20 Dec 2023 12:11:26 +0000 Subject: [PATCH 15/19] Add example as recognised key pt4 Signed-off-by: Ahdra Merali --- kedro/framework/startup.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/kedro/framework/startup.py b/kedro/framework/startup.py index 37aa27819a..adbb7b19f2 100644 --- a/kedro/framework/startup.py +++ b/kedro/framework/startup.py @@ -22,6 +22,7 @@ class ProjectMetadata(NamedTuple): source_dir: Path kedro_init_version: str tools: list + example_pipeline: str def _version_mismatch_error(kedro_init_version) -> str: @@ -103,6 +104,7 @@ def _get_project_metadata(project_path: Union[str, Path]) -> ProjectMetadata: source_dir = Path(metadata_dict.get("source_dir", "src")).expanduser() source_dir = (project_path / source_dir).resolve() metadata_dict["tools"] = metadata_dict.get("tools") + metadata_dict["example_pipeline"] = metadata_dict.get("example_pipeline") metadata_dict["source_dir"] = source_dir metadata_dict["config_file"] = pyproject_toml From 40558d45c2453e2fde55f7888c33a778d90968dc Mon Sep 17 00:00:00 2001 From: Ahdra Merali Date: Wed, 20 Dec 2023 12:19:41 +0000 Subject: [PATCH 16/19] Add example as recognised key pt5 Signed-off-by: Ahdra Merali --- tests/framework/test_startup.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/framework/test_startup.py b/tests/framework/test_startup.py index d8de98315a..4154dfe497 100644 --- a/tests/framework/test_startup.py +++ b/tests/framework/test_startup.py @@ -95,6 +95,7 @@ def test_valid_toml_file(self, mocker): kedro_init_version=kedro_version, project_path=self.project_path, tools=None, + example_pipeline=None, ) assert actual == expected @@ -122,6 +123,7 @@ def test_valid_toml_file_with_project_version(self, mocker): kedro_init_version=kedro_version, project_path=self.project_path, tools=None, + example_pipeline=None, ) assert actual == expected @@ -301,6 +303,7 @@ def test_bootstrap_project(self, monkeypatch, tmp_path): "kedro_init_version": kedro_version, "source_dir": src_dir, "tools": None, + "example_pipeline": None, } assert result == ProjectMetadata(**expected_metadata) assert str(src_dir) in sys.path[0] From 4220d2a55095c8afa395cd2d1494c76e3717ce3e Mon Sep 17 00:00:00 2001 From: Ahdra Merali Date: Wed, 20 Dec 2023 12:26:46 +0000 Subject: [PATCH 17/19] Add example as recognised key pt6 Signed-off-by: Ahdra Merali --- tests/framework/cli/conftest.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/framework/cli/conftest.py b/tests/framework/cli/conftest.py index a151136092..7aae7ff4a2 100644 --- a/tests/framework/cli/conftest.py +++ b/tests/framework/cli/conftest.py @@ -89,6 +89,7 @@ def fake_metadata(fake_root_dir): kedro_init_version=kedro_version, source_dir=fake_root_dir / REPO_NAME / "src", tools=None, + example_pipeline=None, ) return metadata From 3a2f1fe48972164f9d9f3c99ebc914b441eafac6 Mon Sep 17 00:00:00 2001 From: Ahdra Merali Date: Wed, 20 Dec 2023 12:41:15 +0000 Subject: [PATCH 18/19] Add example as recognised key pt7 Signed-off-by: Ahdra Merali --- tests/ipython/test_ipython.py | 1 + tests/tools/test_cli.py | 15 ++++++++------- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/tests/ipython/test_ipython.py b/tests/ipython/test_ipython.py index 20771565d4..3871803bb2 100644 --- a/tests/ipython/test_ipython.py +++ b/tests/ipython/test_ipython.py @@ -39,6 +39,7 @@ def fake_metadata(tmp_path): kedro_init_version=PROJECT_INIT_VERSION, project_path=tmp_path, tools=None, + example_pipeline=None, ) return metadata diff --git a/tests/tools/test_cli.py b/tests/tools/test_cli.py index b50778c6a3..ece4c7054d 100644 --- a/tests/tools/test_cli.py +++ b/tests/tools/test_cli.py @@ -37,13 +37,14 @@ def fake_root_dir(tmp_path): @pytest.fixture def fake_metadata(fake_root_dir): metadata = ProjectMetadata( - fake_root_dir / REPO_NAME / "pyproject.toml", - PACKAGE_NAME, - "CLI Tools Testing Project", - fake_root_dir / REPO_NAME, - kedro_version, - fake_root_dir / REPO_NAME / "src", - kedro_version, + source_dir=fake_root_dir / REPO_NAME / "src", + config_file=fake_root_dir / REPO_NAME / "pyproject.toml", + package_name=PACKAGE_NAME, + project_name="CLI Tools Testing Project", + kedro_init_version=kedro_version, + project_path=fake_root_dir / REPO_NAME, + tools=None, + example_pipeline=None, ) return metadata From f0ca34fbadd8fee939c885ba9f7f19fd12c5891a Mon Sep 17 00:00:00 2001 From: Ahdra Merali Date: Wed, 20 Dec 2023 15:18:47 +0000 Subject: [PATCH 19/19] Streamline condition Signed-off-by: Ahdra Merali --- kedro/framework/cli/starters.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/kedro/framework/cli/starters.py b/kedro/framework/cli/starters.py index b43175dfaa..21f1766a1d 100644 --- a/kedro/framework/cli/starters.py +++ b/kedro/framework/cli/starters.py @@ -538,15 +538,13 @@ def _convert_tool_names_to_numbers(selected_tools: str | None) -> str | None: Args: selected_tools: a string containing the value for the --tools flag, - or None in case the flag wasn't used, i.e. lint,docs. + or None in case none were provided, i.e. lint,docs. Returns: String with the numbers corresponding to the desired tools, or None in case the --tools flag was not used. """ - if selected_tools is None: - return None - if selected_tools.lower() == "none": + if selected_tools is None or selected_tools.lower() == "none": return None if selected_tools.lower() == "all": return ",".join(NUMBER_TO_TOOLS_NAME.keys())