From f7f4d6ab17b5c4c1ff267edf3e368b1b70c395ba Mon Sep 17 00:00:00 2001 From: Nok Date: Mon, 13 Nov 2023 04:36:02 +0000 Subject: [PATCH 01/10] modify release note Signed-off-by: Nok --- RELEASE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RELEASE.md b/RELEASE.md index 33dc1c28ae..a0c0057e7d 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -478,7 +478,7 @@ We are grateful to the following for submitting PRs that contributed to this rel ## Bug fixes and other changes * Removed fatal error from being logged when a Kedro session is created in a directory without git. -* `KedroContext` is now an `attrs`'s frozen class and `config_loader` is available as public attribute. +* `KedroContext`'s attributes are now frozen and `config_loader` is available as public attribute. * Fixed `CONFIG_LOADER_CLASS` validation so that `TemplatedConfigLoader` can be specified in settings.py. Any `CONFIG_LOADER_CLASS` must be a subclass of `AbstractConfigLoader`. * Added runner name to the `run_params` dictionary used in pipeline hooks. * Updated [Databricks documentation](https://docs.kedro.org/en/0.18.1/deployment/databricks.html) to include how to get it working with IPython extension and Kedro-Viz. From bc85e40adda6efcbe7139902f5c948fe83cbdb11 Mon Sep 17 00:00:00 2001 From: Nok Date: Mon, 13 Nov 2023 05:16:45 +0000 Subject: [PATCH 02/10] Make KedroContext partial frozen Signed-off-by: Nok --- kedro/framework/context/context.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/kedro/framework/context/context.py b/kedro/framework/context/context.py index cc55038f40..5a3648c644 100644 --- a/kedro/framework/context/context.py +++ b/kedro/framework/context/context.py @@ -8,7 +8,8 @@ from urllib.parse import urlparse from warnings import warn -from attrs import field, frozen +from attrs import define, field +from attrs.setters import frozen from pluggy import PluginManager from kedro.config import AbstractConfigLoader, MissingConfigException @@ -158,18 +159,20 @@ def _expand_full_path(project_path: str | Path) -> Path: return Path(project_path).expanduser().resolve() -@frozen +@define(slots=False) # Enable setting new attributes to `KedroContext` class KedroContext: """``KedroContext`` is the base class which holds the configuration and Kedro's main functionality. """ - _package_name: str - project_path: Path = field(converter=_expand_full_path) - config_loader: AbstractConfigLoader - _hook_manager: PluginManager - env: str | None = None - _extra_params: dict[str, Any] | None = field(default=None, converter=deepcopy) + _package_name: str = field(init=True, on_setattr=frozen) + project_path: Path = field(init=True, converter=_expand_full_path) + config_loader: AbstractConfigLoader = field(init=True, on_setattr=frozen) + _hook_manager: PluginManager = field(init=True, on_setattr=frozen) + env: str | None = field(init=True, on_setattr=frozen) + _extra_params: dict[str, Any] | None = field( + init=True, default=None, converter=deepcopy, on_setattr=frozen + ) """Create a context object by providing the root of a Kedro project and the environment configuration subfolders (see ``kedro.config.OmegaConfigLoader``) From ee95c3b0d59a9294f7f1d23e08d4e4c39ad78050 Mon Sep 17 00:00:00 2001 From: Nok Date: Mon, 13 Nov 2023 06:57:24 +0000 Subject: [PATCH 03/10] fix KedroContext and add test for public and internal attributes Signed-off-by: Nok --- kedro/framework/context/context.py | 4 +++- tests/framework/context/test_context.py | 28 +++++++++++++++++++++---- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/kedro/framework/context/context.py b/kedro/framework/context/context.py index 5a3648c644..9f2172cab0 100644 --- a/kedro/framework/context/context.py +++ b/kedro/framework/context/context.py @@ -166,7 +166,9 @@ class KedroContext: """ _package_name: str = field(init=True, on_setattr=frozen) - project_path: Path = field(init=True, converter=_expand_full_path) + project_path: Path = field( + init=True, converter=_expand_full_path, on_setattr=frozen + ) config_loader: AbstractConfigLoader = field(init=True, on_setattr=frozen) _hook_manager: PluginManager = field(init=True, on_setattr=frozen) env: str | None = field(init=True, on_setattr=frozen) diff --git a/tests/framework/context/test_context.py b/tests/framework/context/test_context.py index f5dc49e81b..a3b72d2f99 100644 --- a/tests/framework/context/test_context.py +++ b/tests/framework/context/test_context.py @@ -12,7 +12,7 @@ import pytest import toml import yaml -from attrs.exceptions import FrozenInstanceError +from attrs.exceptions import FrozenAttributeError from pandas.testing import assert_frame_equal from kedro import __version__ as kedro_version @@ -209,9 +209,29 @@ def test_attributes(self, tmp_path, dummy_context): assert isinstance(dummy_context.project_path, Path) assert dummy_context.project_path == tmp_path.resolve() - def test_immutable_instance(self, dummy_context): - with pytest.raises(FrozenInstanceError): - dummy_context.catalog = 1 + @pytest.mark.parametrize( + "attr", + ( + "project_path", + "config_loader", + "env", + ), + ) + def test_public_attributes(self, dummy_context, attr): + getattr(dummy_context, attr) + + @pytest.mark.parametrize( + "internal_attr", ("_package_name", "_hook_manager", "_extra_params") + ) + def test_internal_attributes(self, dummy_context, internal_attr): + getattr(dummy_context, internal_attr) + + def test_immutable_class_attribute(self, dummy_context): + with pytest.raises(FrozenAttributeError): + dummy_context.project_path = "dummy" + + def test_set_new_attribute(self, dummy_context): + dummy_context.mlflow = 1 def test_get_catalog_always_using_absolute_path(self, dummy_context): config_loader = dummy_context.config_loader From ee1048a9e87c59cd1c826744f198d779db3073fd Mon Sep 17 00:00:00 2001 From: Nok Date: Tue, 14 Nov 2023 06:39:56 +0000 Subject: [PATCH 04/10] Fix tests Signed-off-by: Nok --- tests/framework/session/test_session.py | 47 +++++++++++++++++++++++-- 1 file changed, 44 insertions(+), 3 deletions(-) diff --git a/tests/framework/session/test_session.py b/tests/framework/session/test_session.py index b1afc11e7f..7bda29beb6 100644 --- a/tests/framework/session/test_session.py +++ b/tests/framework/session/test_session.py @@ -1,9 +1,12 @@ import logging import re import subprocess +import sys import textwrap from collections.abc import Mapping from pathlib import Path +from typing import Any, Type +from unittest.mock import create_autospec import pytest import toml @@ -13,7 +16,6 @@ from kedro import __version__ as kedro_version from kedro.config import AbstractConfigLoader, OmegaConfigLoader from kedro.framework.cli.utils import _split_params -from kedro.framework.context import KedroContext from kedro.framework.project import ( LOGGING, ValidationError, @@ -23,7 +25,7 @@ _ProjectSettings, ) from kedro.framework.session import KedroSession -from kedro.framework.session.session import KedroSessionError +from kedro.framework.session.session import KedroContext, KedroSessionError from kedro.framework.session.shelvestore import ShelveStore from kedro.framework.session.store import BaseSessionStore @@ -43,6 +45,36 @@ class BadConfigLoader: """ +ATTRS_ATTRIBUTE = "__attrs_attrs__" + +NEW_TYPING = sys.version_info[:3] >= (3, 7, 0) # PEP 560 + + +def create_attrs_autospec(spec: Type, spec_set: bool = True) -> Any: + """Creates a mock of an attr class (creates mocks recursively on all attributes). + https://github.com/python-attrs/attrs/issues/462#issuecomment-1134656377 + + :param spec: the spec to mock + :param spec_set: if True, AttributeError will be raised if an attribute that is not in the spec is set. + """ + + if not hasattr(spec, ATTRS_ATTRIBUTE): + raise TypeError(f"{spec!r} is not an attrs class") + mock = create_autospec(spec, spec_set=spec_set) + for attribute in getattr(spec, ATTRS_ATTRIBUTE): + attribute_type = attribute.type + if NEW_TYPING: + # A[T] does not get a copy of __dict__ from A(Generic[T]) anymore, use __origin__ to get it + while hasattr(attribute_type, "__origin__"): + attribute_type = attribute_type.__origin__ + if hasattr(attribute_type, ATTRS_ATTRIBUTE): + mock_attribute = create_attrs_autospec(attribute_type, spec_set) + else: + mock_attribute = create_autospec(attribute_type, spec_set=spec_set) + object.__setattr__(mock, attribute.name, mock_attribute) + return mock + + @pytest.fixture def mock_runner(mocker): mock_runner = mocker.patch( @@ -55,7 +87,12 @@ def mock_runner(mocker): @pytest.fixture def mock_context_class(mocker): - return mocker.patch("kedro.framework.session.session.KedroContext", autospec=True) + mock_cls = create_attrs_autospec(KedroContext) + return mocker.patch( + "kedro.framework.session.session.KedroContext", + autospec=True, + return_value=mock_cls, + ) def _mock_imported_settings_paths(mocker, mock_settings): @@ -74,7 +111,9 @@ def mock_settings(mocker): @pytest.fixture def mock_settings_context_class(mocker, mock_context_class): + # mocker.patch("dynaconf.base.LazySettings.unset") class MockSettings(_ProjectSettings): + # dynaconf automatically deleted some attribute when the class is MagicMock _CONTEXT_CLASS = Validator( "CONTEXT_CLASS", default=lambda *_: mock_context_class ) @@ -601,7 +640,9 @@ def test_run( "__default__": mocker.Mock(), }, ) + print(f"{mock_context_class=}") mock_context = mock_context_class.return_value + print(f"{mock_context=}") mock_catalog = mock_context._get_catalog.return_value mock_runner.__name__ = "SequentialRunner" mock_pipeline = mock_pipelines.__getitem__.return_value.filter.return_value From 59e639d7d3b383f20ba0cafac241d2f4934f4c89 Mon Sep 17 00:00:00 2001 From: Nok Date: Mon, 20 Nov 2023 08:41:31 +0000 Subject: [PATCH 05/10] Rearrange the `KedroContext` arguments Signed-off-by: Nok --- kedro/framework/context/context.py | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/kedro/framework/context/context.py b/kedro/framework/context/context.py index 4c61f9d0c8..1bfe390404 100644 --- a/kedro/framework/context/context.py +++ b/kedro/framework/context/context.py @@ -163,15 +163,33 @@ def _expand_full_path(project_path: str | Path) -> Path: class KedroContext: """``KedroContext`` is the base class which holds the configuration and Kedro's main functionality. + + Create a context object by providing the root of a Kedro project and + the environment configuration subfolders (see ``kedro.config.OmegaConfigLoader``) + Raises: + KedroContextError: If there is a mismatch + between Kedro project version and package version. + Args: + project_path: Project path to define the context for. + config_loader: Kedro's ``OmegaConfigLoader`` for loading the configuration files. + package_name: Package name for the Kedro project the context is + created for. + env: Optional argument for configuration default environment to be used + for running the pipeline. If not specified, it defaults to "local". + hook_manager: The ``PluginManager`` to activate hooks, supplied by the session. + extra_params: Optional dictionary containing extra project parameters. + If specified, will update (and therefore take precedence over) + the parameters retrieved from the project configuration. + """ - _package_name: str = field(init=True, on_setattr=frozen) project_path: Path = field( init=True, converter=_expand_full_path, on_setattr=frozen ) config_loader: AbstractConfigLoader = field(init=True, on_setattr=frozen) - _hook_manager: PluginManager = field(init=True, on_setattr=frozen) env: str | None = field(init=True, on_setattr=frozen) + _package_name: str = field(init=True, on_setattr=frozen) + _hook_manager: PluginManager = field(init=True, on_setattr=frozen) _extra_params: dict[str, Any] | None = field( init=True, default=None, converter=deepcopy, on_setattr=frozen ) From 8e4ea178145e9bd5f986a61e6457c059bc55cb30 Mon Sep 17 00:00:00 2001 From: Nok Date: Mon, 20 Nov 2023 08:43:18 +0000 Subject: [PATCH 06/10] improve test_set_new_attribute Signed-off-by: Nok --- tests/framework/context/test_context.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/framework/context/test_context.py b/tests/framework/context/test_context.py index a3b72d2f99..e933ecb313 100644 --- a/tests/framework/context/test_context.py +++ b/tests/framework/context/test_context.py @@ -232,6 +232,7 @@ def test_immutable_class_attribute(self, dummy_context): def test_set_new_attribute(self, dummy_context): dummy_context.mlflow = 1 + assert dummy_context.mlflow == 1 def test_get_catalog_always_using_absolute_path(self, dummy_context): config_loader = dummy_context.config_loader From b274927bd6fa0539dd71c58db821bb5650b1630e Mon Sep 17 00:00:00 2001 From: Nok Date: Mon, 20 Nov 2023 08:58:41 +0000 Subject: [PATCH 07/10] Unfreeze the Kedrocontext attributes Signed-off-by: Nok --- kedro/framework/context/context.py | 15 ++++++--------- tests/framework/context/test_context.py | 11 +++-------- 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/kedro/framework/context/context.py b/kedro/framework/context/context.py index 1bfe390404..5ede14bc63 100644 --- a/kedro/framework/context/context.py +++ b/kedro/framework/context/context.py @@ -9,7 +9,6 @@ from warnings import warn from attrs import define, field -from attrs.setters import frozen from pluggy import PluginManager from kedro.config import AbstractConfigLoader, MissingConfigException @@ -183,15 +182,13 @@ class KedroContext: """ - project_path: Path = field( - init=True, converter=_expand_full_path, on_setattr=frozen - ) - config_loader: AbstractConfigLoader = field(init=True, on_setattr=frozen) - env: str | None = field(init=True, on_setattr=frozen) - _package_name: str = field(init=True, on_setattr=frozen) - _hook_manager: PluginManager = field(init=True, on_setattr=frozen) + project_path: Path = field(init=True, converter=_expand_full_path) + config_loader: AbstractConfigLoader = field(init=True) + env: str | None = field(init=True) + _package_name: str = field(init=True) + _hook_manager: PluginManager = field(init=True) _extra_params: dict[str, Any] | None = field( - init=True, default=None, converter=deepcopy, on_setattr=frozen + init=True, default=None, converter=deepcopy ) @property diff --git a/tests/framework/context/test_context.py b/tests/framework/context/test_context.py index e933ecb313..a7604c443f 100644 --- a/tests/framework/context/test_context.py +++ b/tests/framework/context/test_context.py @@ -12,7 +12,6 @@ import pytest import toml import yaml -from attrs.exceptions import FrozenAttributeError from pandas.testing import assert_frame_equal from kedro import __version__ as kedro_version @@ -192,11 +191,11 @@ def dummy_context(tmp_path, prepare_project_dir, env, extra_params): configure_project(MOCK_PACKAGE_NAME) config_loader = OmegaConfigLoader(str(tmp_path / "conf"), env=env) context = KedroContext( - MOCK_PACKAGE_NAME, - str(tmp_path), + project_path=str(tmp_path), config_loader=config_loader, - hook_manager=_create_hook_manager(), env=env, + package_name=MOCK_PACKAGE_NAME, + hook_manager=_create_hook_manager(), extra_params=extra_params, ) @@ -226,10 +225,6 @@ def test_public_attributes(self, dummy_context, attr): def test_internal_attributes(self, dummy_context, internal_attr): getattr(dummy_context, internal_attr) - def test_immutable_class_attribute(self, dummy_context): - with pytest.raises(FrozenAttributeError): - dummy_context.project_path = "dummy" - def test_set_new_attribute(self, dummy_context): dummy_context.mlflow = 1 assert dummy_context.mlflow == 1 From 7553f3a684cc7a096c9d1fe9184ed2ed09635a01 Mon Sep 17 00:00:00 2001 From: Nok Date: Tue, 21 Nov 2023 04:41:17 +0000 Subject: [PATCH 08/10] clean up Signed-off-by: Nok --- RELEASE.md | 2 +- tests/framework/session/test_session.py | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/RELEASE.md b/RELEASE.md index f8ab95a6cd..81e9972758 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -479,7 +479,7 @@ We are grateful to the following for submitting PRs that contributed to this rel ## Bug fixes and other changes * Removed fatal error from being logged when a Kedro session is created in a directory without git. -* `KedroContext`'s attributes are now frozen and `config_loader` is available as public attribute. +* `KedroContext` is now a attr's dataclass, `config_loader` is available as public attribute. * Fixed `CONFIG_LOADER_CLASS` validation so that `TemplatedConfigLoader` can be specified in settings.py. Any `CONFIG_LOADER_CLASS` must be a subclass of `AbstractConfigLoader`. * Added runner name to the `run_params` dictionary used in pipeline hooks. * Updated [Databricks documentation](https://docs.kedro.org/en/0.18.1/deployment/databricks.html) to include how to get it working with IPython extension and Kedro-Viz. diff --git a/tests/framework/session/test_session.py b/tests/framework/session/test_session.py index 7bda29beb6..9e4e9364a5 100644 --- a/tests/framework/session/test_session.py +++ b/tests/framework/session/test_session.py @@ -111,7 +111,6 @@ def mock_settings(mocker): @pytest.fixture def mock_settings_context_class(mocker, mock_context_class): - # mocker.patch("dynaconf.base.LazySettings.unset") class MockSettings(_ProjectSettings): # dynaconf automatically deleted some attribute when the class is MagicMock _CONTEXT_CLASS = Validator( @@ -640,9 +639,7 @@ def test_run( "__default__": mocker.Mock(), }, ) - print(f"{mock_context_class=}") mock_context = mock_context_class.return_value - print(f"{mock_context=}") mock_catalog = mock_context._get_catalog.return_value mock_runner.__name__ = "SequentialRunner" mock_pipeline = mock_pipelines.__getitem__.return_value.filter.return_value From 3ce19cc852643b344d363ee2a315a6607e2781f7 Mon Sep 17 00:00:00 2001 From: Nok Date: Tue, 21 Nov 2023 04:51:00 +0000 Subject: [PATCH 09/10] remove redundant test Signed-off-by: Nok --- kedro/framework/context/context.py | 4 ++-- tests/framework/context/test_context.py | 17 ----------------- 2 files changed, 2 insertions(+), 19 deletions(-) diff --git a/kedro/framework/context/context.py b/kedro/framework/context/context.py index 5ede14bc63..2034880f41 100644 --- a/kedro/framework/context/context.py +++ b/kedro/framework/context/context.py @@ -171,10 +171,10 @@ class KedroContext: Args: project_path: Project path to define the context for. config_loader: Kedro's ``OmegaConfigLoader`` for loading the configuration files. - package_name: Package name for the Kedro project the context is - created for. env: Optional argument for configuration default environment to be used for running the pipeline. If not specified, it defaults to "local". + package_name: Package name for the Kedro project the context is + created for. hook_manager: The ``PluginManager`` to activate hooks, supplied by the session. extra_params: Optional dictionary containing extra project parameters. If specified, will update (and therefore take precedence over) diff --git a/tests/framework/context/test_context.py b/tests/framework/context/test_context.py index a7604c443f..a7b23a4831 100644 --- a/tests/framework/context/test_context.py +++ b/tests/framework/context/test_context.py @@ -208,23 +208,6 @@ def test_attributes(self, tmp_path, dummy_context): assert isinstance(dummy_context.project_path, Path) assert dummy_context.project_path == tmp_path.resolve() - @pytest.mark.parametrize( - "attr", - ( - "project_path", - "config_loader", - "env", - ), - ) - def test_public_attributes(self, dummy_context, attr): - getattr(dummy_context, attr) - - @pytest.mark.parametrize( - "internal_attr", ("_package_name", "_hook_manager", "_extra_params") - ) - def test_internal_attributes(self, dummy_context, internal_attr): - getattr(dummy_context, internal_attr) - def test_set_new_attribute(self, dummy_context): dummy_context.mlflow = 1 assert dummy_context.mlflow == 1 From cf719368d8fd12269e4e48f19af3b275356469d9 Mon Sep 17 00:00:00 2001 From: Nok Date: Tue, 21 Nov 2023 16:30:09 +0000 Subject: [PATCH 10/10] fix import Signed-off-by: Nok --- kedro/framework/context/context.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/kedro/framework/context/context.py b/kedro/framework/context/context.py index 253d149413..919895bc34 100644 --- a/kedro/framework/context/context.py +++ b/kedro/framework/context/context.py @@ -8,12 +8,8 @@ from urllib.parse import urlparse from warnings import warn - from attrs import define, field - - -p - +from omegaconf import OmegaConf from pluggy import PluginManager from kedro.config import AbstractConfigLoader, MissingConfigException