From 28e8eb0e65e2dd599c04c59e4396f747e4ea6fc8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 23:38:20 +0000 Subject: [PATCH 1/4] Initial plan From 5cda0b05c16904a1ad9fb17b9db01a7393fe8fec Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 23:46:11 +0000 Subject: [PATCH 2/4] Fix test_mode string evaluation in BaseMixin._test_mode property - Changed _test_mode property to use spyglass.settings.config.test_mode instead of reading directly from dj.config - This ensures proper string-to-boolean conversion via str_to_bool() function - Added test to verify _test_mode property returns boolean value - Added comprehensive unit test for str_to_bool function Fixes #1528 Co-authored-by: samuelbray32 <24991442+samuelbray32@users.noreply.github.com> --- src/spyglass/utils/mixins/base.py | 8 +++---- tests/utils/test_dj_helper_fn.py | 39 +++++++++++++++++++++++++++++++ tests/utils/test_mixin.py | 22 +++++++++++++++++ 3 files changed, 64 insertions(+), 5 deletions(-) diff --git a/src/spyglass/utils/mixins/base.py b/src/spyglass/utils/mixins/base.py index b9c7ed2d2..d72da7a3b 100644 --- a/src/spyglass/utils/mixins/base.py +++ b/src/spyglass/utils/mixins/base.py @@ -43,17 +43,15 @@ def _test_mode(self) -> bool: Avoids circular import. Prevents prompt on delete. Note: Using @property instead of @cached_property so we always get - current value from dj.config, even if test_mode changes after first access. + current value from settings, even if test_mode changes after first access. Used by ... - BaseMixin._spyglass_version - HelpersMixin """ - import datajoint as dj + from spyglass.settings import config as sg_config - # Check dj.config directly instead of importing module-level variable - # which gets stale if load_config() is called after initial import - return dj.config.get("custom", {}).get("test_mode", False) + return sg_config.test_mode @cached_property def _spyglass_version(self): diff --git a/tests/utils/test_dj_helper_fn.py b/tests/utils/test_dj_helper_fn.py index aa063648f..c688e04fd 100644 --- a/tests/utils/test_dj_helper_fn.py +++ b/tests/utils/test_dj_helper_fn.py @@ -9,3 +9,42 @@ def test_deprecation_factory(caplog, common): assert ( "Deprecation:" in caplog.text ), "No deprecation warning logged on migrated table." + + +def test_str_to_bool(): + """Test str_to_bool function handles various string inputs correctly. + + This test verifies that str_to_bool properly converts string values + to booleans, which is essential for fixing issue #1528 where string + "false" was incorrectly evaluated as True. + """ + from spyglass.utils.dj_helper_fn import str_to_bool + + # Test truthy strings (case insensitive) + assert str_to_bool("true") is True + assert str_to_bool("True") is True + assert str_to_bool("TRUE") is True + assert str_to_bool("t") is True + assert str_to_bool("T") is True + assert str_to_bool("yes") is True + assert str_to_bool("YES") is True + assert str_to_bool("y") is True + assert str_to_bool("Y") is True + assert str_to_bool("1") is True + + # Test falsy strings + assert str_to_bool("false") is False + assert str_to_bool("False") is False + assert str_to_bool("FALSE") is False + assert str_to_bool("no") is False + assert str_to_bool("n") is False + assert str_to_bool("0") is False + assert str_to_bool("") is False + assert str_to_bool("random_string") is False + + # Test non-string inputs + assert str_to_bool(True) is True + assert str_to_bool(False) is False + assert str_to_bool(None) is False + assert str_to_bool(0) is False + assert str_to_bool(1) is True diff --git a/tests/utils/test_mixin.py b/tests/utils/test_mixin.py index 9197c67b7..dab30e96e 100644 --- a/tests/utils/test_mixin.py +++ b/tests/utils/test_mixin.py @@ -173,3 +173,25 @@ def test_mixin_del_orphans(dj_conn, Mixin, MixinChild): Mixin().delete_orphans(dry_run=False, safemode=False) post_del = Mixin().fetch1("id") assert post_del == 0, "Delete orphans not working." + + +def test_test_mode_property_uses_settings(schema_test, Mixin): + """Test that _test_mode property uses spyglass.settings.config. + + Verifies fix for issue #1528 where string "false" was incorrectly + evaluated as True. The property should now use SpyglassConfig.test_mode + which properly converts strings to booleans via str_to_bool(). + """ + schema_test(Mixin) + + # The _test_mode property should return a boolean + test_mode_value = Mixin()._test_mode + assert isinstance(test_mode_value, bool), ( + "_test_mode should return a boolean value" + ) + + # In test environment, test_mode should be True + # (set by load_config fixture in conftest.py) + assert test_mode_value is True, ( + "_test_mode should be True in test environment" + ) From cb4340b89c43bf382c74f61de4158c2fc4ab57cd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 23:47:40 +0000 Subject: [PATCH 3/4] Update test docstring for clarity Changed docstring to reference exact module path 'spyglass.settings.config.test_mode' instead of 'SpyglassConfig.test_mode' Co-authored-by: samuelbray32 <24991442+samuelbray32@users.noreply.github.com> --- tests/utils/test_mixin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/utils/test_mixin.py b/tests/utils/test_mixin.py index dab30e96e..afde038c7 100644 --- a/tests/utils/test_mixin.py +++ b/tests/utils/test_mixin.py @@ -179,7 +179,7 @@ def test_test_mode_property_uses_settings(schema_test, Mixin): """Test that _test_mode property uses spyglass.settings.config. Verifies fix for issue #1528 where string "false" was incorrectly - evaluated as True. The property should now use SpyglassConfig.test_mode + evaluated as True. The property should now use spyglass.settings.config.test_mode which properly converts strings to booleans via str_to_bool(). """ schema_test(Mixin) From 248315de520e54a509adc2ebe0e92b25753e7d5d Mon Sep 17 00:00:00 2001 From: samuelbray32 Date: Fri, 20 Feb 2026 08:36:47 -0800 Subject: [PATCH 4/4] fix import of test_mode --- src/spyglass/utils/mixins/base.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/spyglass/utils/mixins/base.py b/src/spyglass/utils/mixins/base.py index d72da7a3b..e10791dfb 100644 --- a/src/spyglass/utils/mixins/base.py +++ b/src/spyglass/utils/mixins/base.py @@ -36,22 +36,19 @@ def _graph_deps(self) -> list: return [TableChain, RestrGraph] - @property + @cached_property def _test_mode(self) -> bool: """Return True if in test mode. Avoids circular import. Prevents prompt on delete. - Note: Using @property instead of @cached_property so we always get - current value from settings, even if test_mode changes after first access. - Used by ... - BaseMixin._spyglass_version - HelpersMixin """ - from spyglass.settings import config as sg_config + from spyglass.settings import test_mode - return sg_config.test_mode + return test_mode @cached_property def _spyglass_version(self):