From b333b3610a0dd365547bd4c6d4b815dfda46fd71 Mon Sep 17 00:00:00 2001 From: Keisuke Ogaki Date: Fri, 17 Nov 2023 20:03:00 +0900 Subject: [PATCH 1/5] :white_check_mark: test PandasTypeCheck and build --- test/test_pandas_type_check_framework.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/test/test_pandas_type_check_framework.py b/test/test_pandas_type_check_framework.py index c36fb99d..49367ea1 100644 --- a/test/test_pandas_type_check_framework.py +++ b/test/test_pandas_type_check_framework.py @@ -2,6 +2,7 @@ from logging import getLogger from typing import Any, Dict from unittest.mock import patch +from gokart.build import GokartBuildError import luigi import pandas as pd @@ -68,23 +69,22 @@ def tearDown(self) -> None: luigi.setup_logging.DaemonLogging._configured = False luigi.setup_logging.InterfaceLogging._configured = False - @patch('sys.argv', new=['main', 'test_pandas_type_check_framework._DummyFailTask', '--log-level=CRITICAL', '--local-scheduler', '--no-lock']) + + @patch('sys.argv', new=['main', 'test_pandas_type_check_framework._DummyFailTask', '--log-level=CRITICAL', '--local-scheduler', '--no-lock']) @patch('luigi.LocalTarget', new=lambda path, **kwargs: MockTarget(path, **kwargs)) - def test_fail(self): + def test_fail_with_gokart_run(self): with self.assertRaises(SystemExit) as exit_code: gokart.run() self.assertNotEqual(exit_code.exception.code, 0) # raise Error - @patch('sys.argv', new=['main', 'test_pandas_type_check_framework._DummyFailWithNoneTask', '--log-level=CRITICAL', '--local-scheduler', '--no-lock']) - @patch('luigi.LocalTarget', new=lambda path, **kwargs: MockTarget(path, **kwargs)) + def test_fail(self): + with self.assertRaises(GokartBuildError): + gokart.build(_DummyFailTask()) + def test_fail_with_None(self): - with self.assertRaises(SystemExit) as exit_code: - gokart.run() - self.assertNotEqual(exit_code.exception.code, 0) # raise Error + with self.assertRaises(GokartBuildError): + gokart.build(_DummyFailWithNoneTask()) - @patch('sys.argv', new=['main', 'test_pandas_type_check_framework._DummySuccessTask', '--log-level=CRITICAL', '--local-scheduler', '--no-lock']) - @patch('luigi.LocalTarget', new=lambda path, **kwargs: MockTarget(path, **kwargs)) def test_success(self): - with self.assertRaises(SystemExit) as exit_code: - gokart.run() - self.assertEqual(exit_code.exception.code, 0) + gokart.build(_DummySuccessTask()) + # no error From f406cb8d1bd2a0d068717a10edf5bda4009dffee Mon Sep 17 00:00:00 2001 From: Keisuke Ogaki Date: Fri, 17 Nov 2023 20:17:03 +0900 Subject: [PATCH 2/5] :art: do not reset PandasTypeConfig when reset_register --- gokart/build.py | 9 +++++++-- test/test_pandas_type_check_framework.py | 9 +++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/gokart/build.py b/gokart/build.py index 7662cfa5..d007bb86 100644 --- a/gokart/build.py +++ b/gokart/build.py @@ -3,6 +3,7 @@ from typing import Any, Optional import luigi +import gokart from gokart.task import TaskOnKart @@ -38,8 +39,12 @@ def _get_output(task: TaskOnKart) -> Any: def _reset_register(keep={'gokart', 'luigi'}): - luigi.task_register.Register._reg = [x for x in luigi.task_register.Register._reg - if x.__module__.split('.')[0] in keep] # avoid TaskClassAmbigiousException + """reset luigi.task_register.Register._reg everytime gokart.build called to avoid TaskClassAmbigiousException""" + luigi.task_register.Register._reg = [ + x for x in luigi.task_register.Register._reg if ((x.__module__.split('.')[0] in keep) # keep luigi and gokart + or (issubclass(x, gokart.PandasTypeConfig)) # PandasTypeConfig should be kept + ) + ] def build(task: TaskOnKart, return_value: bool = True, reset_register: bool = True, log_level: int = logging.ERROR, **env_params) -> Optional[Any]: diff --git a/test/test_pandas_type_check_framework.py b/test/test_pandas_type_check_framework.py index 49367ea1..ac397f06 100644 --- a/test/test_pandas_type_check_framework.py +++ b/test/test_pandas_type_check_framework.py @@ -69,8 +69,7 @@ def tearDown(self) -> None: luigi.setup_logging.DaemonLogging._configured = False luigi.setup_logging.InterfaceLogging._configured = False - - @patch('sys.argv', new=['main', 'test_pandas_type_check_framework._DummyFailTask', '--log-level=CRITICAL', '--local-scheduler', '--no-lock']) + @patch('sys.argv', new=['main', 'test_pandas_type_check_framework._DummyFailTask', '--log-level=CRITICAL', '--local-scheduler', '--no-lock']) @patch('luigi.LocalTarget', new=lambda path, **kwargs: MockTarget(path, **kwargs)) def test_fail_with_gokart_run(self): with self.assertRaises(SystemExit) as exit_code: @@ -78,13 +77,19 @@ def test_fail_with_gokart_run(self): self.assertNotEqual(exit_code.exception.code, 0) # raise Error def test_fail(self): + original_reg = luigi.task_register.Register._reg with self.assertRaises(GokartBuildError): gokart.build(_DummyFailTask()) + luigi.task_register.Register._reg = original_reg def test_fail_with_None(self): + original_reg = luigi.task_register.Register._reg with self.assertRaises(GokartBuildError): gokart.build(_DummyFailWithNoneTask()) + luigi.task_register.Register._reg = original_reg def test_success(self): + original_reg = luigi.task_register.Register._reg gokart.build(_DummySuccessTask()) # no error + luigi.task_register.Register._reg = original_reg From 74f6e60c4446ec1f4f324554ca986ff19a89ca6d Mon Sep 17 00:00:00 2001 From: Keisuke Ogaki Date: Fri, 17 Nov 2023 20:21:20 +0900 Subject: [PATCH 3/5] :art: better register reset in tests 'https://github.com/spotify/luigi/blob/fe7ecf4acf7cf4c084bd0f32162c8e0721567630/test/helpers.py#L175' --- test/test_pandas_type_check_framework.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/test/test_pandas_type_check_framework.py b/test/test_pandas_type_check_framework.py index ac397f06..2d4402bf 100644 --- a/test/test_pandas_type_check_framework.py +++ b/test/test_pandas_type_check_framework.py @@ -64,10 +64,13 @@ def setUp(self) -> None: luigi.setup_logging.DaemonLogging._configured = False luigi.setup_logging.InterfaceLogging._configured = False MockFileSystem().clear() + # same way as luigi https://github.com/spotify/luigi/blob/fe7ecf4acf7cf4c084bd0f32162c8e0721567630/test/helpers.py#L175 + self._stashed_reg = luigi.task_register.Register._get_reg() def tearDown(self) -> None: luigi.setup_logging.DaemonLogging._configured = False luigi.setup_logging.InterfaceLogging._configured = False + luigi.task_register.Register._set_reg(self._stashed_reg) @patch('sys.argv', new=['main', 'test_pandas_type_check_framework._DummyFailTask', '--log-level=CRITICAL', '--local-scheduler', '--no-lock']) @patch('luigi.LocalTarget', new=lambda path, **kwargs: MockTarget(path, **kwargs)) @@ -77,19 +80,13 @@ def test_fail_with_gokart_run(self): self.assertNotEqual(exit_code.exception.code, 0) # raise Error def test_fail(self): - original_reg = luigi.task_register.Register._reg with self.assertRaises(GokartBuildError): gokart.build(_DummyFailTask()) - luigi.task_register.Register._reg = original_reg def test_fail_with_None(self): - original_reg = luigi.task_register.Register._reg with self.assertRaises(GokartBuildError): gokart.build(_DummyFailWithNoneTask()) - luigi.task_register.Register._reg = original_reg def test_success(self): - original_reg = luigi.task_register.Register._reg gokart.build(_DummySuccessTask()) # no error - luigi.task_register.Register._reg = original_reg From 573a143e52cf6ab809c74be115b08b6afa6f8bba Mon Sep 17 00:00:00 2001 From: Keisuke Ogaki Date: Fri, 17 Nov 2023 20:29:35 +0900 Subject: [PATCH 4/5] :shirt: isort and plake8 --- gokart/build.py | 5 ++--- pyproject.toml | 3 ++- test/test_pandas_type_check_framework.py | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/gokart/build.py b/gokart/build.py index d007bb86..68a3a7a9 100644 --- a/gokart/build.py +++ b/gokart/build.py @@ -3,8 +3,8 @@ from typing import Any, Optional import luigi -import gokart +import gokart from gokart.task import TaskOnKart @@ -42,8 +42,7 @@ def _reset_register(keep={'gokart', 'luigi'}): """reset luigi.task_register.Register._reg everytime gokart.build called to avoid TaskClassAmbigiousException""" luigi.task_register.Register._reg = [ x for x in luigi.task_register.Register._reg if ((x.__module__.split('.')[0] in keep) # keep luigi and gokart - or (issubclass(x, gokart.PandasTypeConfig)) # PandasTypeConfig should be kept - ) + or (issubclass(x, gokart.PandasTypeConfig))) # PandasTypeConfig should be kept ] diff --git a/pyproject.toml b/pyproject.toml index a05a29a1..7010e26d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -48,7 +48,8 @@ types-redis = "*" [tool.flake8] # B006: Do not use mutable data structures for argument defaults. They are created during function definition time. All calls to the function reuse this one instance of that data structure, persisting changes between them. # B008 Do not perform function calls in argument defaults. The call is performed only once at function definition time. All calls to your function will reuse the result of that definition-time function call. If this is intended, assign the function call to a module-level variable and use that variable as a default value. -ignore = "B006,B008" +# W503 line break before binary operator. We use W504(line break after binary operator) rather than W503 +ignore = "B006,B008,W503" max-line-length = 160 exclude = "venv/*,tox/*" diff --git a/test/test_pandas_type_check_framework.py b/test/test_pandas_type_check_framework.py index 2d4402bf..a1c79b86 100644 --- a/test/test_pandas_type_check_framework.py +++ b/test/test_pandas_type_check_framework.py @@ -2,13 +2,13 @@ from logging import getLogger from typing import Any, Dict from unittest.mock import patch -from gokart.build import GokartBuildError import luigi import pandas as pd from luigi.mock import MockFileSystem, MockTarget import gokart +from gokart.build import GokartBuildError from gokart.pandas_type_config import PandasTypeConfig logger = getLogger(__name__) From 82478cb26d72020aba28ea70f0c1f70e2327a583 Mon Sep 17 00:00:00 2001 From: Keisuke Ogaki Date: Fri, 17 Nov 2023 20:30:47 +0900 Subject: [PATCH 5/5] :shirt: mute ERRORS on test --- test/test_pandas_type_check_framework.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/test_pandas_type_check_framework.py b/test/test_pandas_type_check_framework.py index a1c79b86..4ec92b2c 100644 --- a/test/test_pandas_type_check_framework.py +++ b/test/test_pandas_type_check_framework.py @@ -1,3 +1,4 @@ +import logging import unittest from logging import getLogger from typing import Any, Dict @@ -81,11 +82,11 @@ def test_fail_with_gokart_run(self): def test_fail(self): with self.assertRaises(GokartBuildError): - gokart.build(_DummyFailTask()) + gokart.build(_DummyFailTask(), log_level=logging.CRITICAL) def test_fail_with_None(self): with self.assertRaises(GokartBuildError): - gokart.build(_DummyFailWithNoneTask()) + gokart.build(_DummyFailWithNoneTask(), log_level=logging.CRITICAL) def test_success(self): gokart.build(_DummySuccessTask())