From 699add37db204a897ad0e6ad8e163b4e4405cb4d Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Thu, 8 May 2025 15:23:24 +0100 Subject: [PATCH 01/10] Add new datatypes. --- datashuttle/configs/canonical_configs.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/datashuttle/configs/canonical_configs.py b/datashuttle/configs/canonical_configs.py index b65ad6c66..374d945cc 100644 --- a/datashuttle/configs/canonical_configs.py +++ b/datashuttle/configs/canonical_configs.py @@ -310,7 +310,8 @@ def get_narrow_datatypes(): The mapping between broad and narrow datatypes is required for validation. """ return { - "ephys": ["ecephys", "icephys"], + "behav": ["motion"], + "ephys": ["ecephys", "icephys", "emg"], "funcimg": ["cscope", "f2pe", "fmri", "fusi"], "anat": [ "2pe", From e44a0043c2b248c5c79deda241c8ea7cfcdf3a79 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Thu, 8 May 2025 16:11:21 +0100 Subject: [PATCH 02/10] Randomly sample datatypes to test. --- tests/tests_integration/test_datatypes.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/tests_integration/test_datatypes.py b/tests/tests_integration/test_datatypes.py index 1e1776e41..67545917f 100644 --- a/tests/tests_integration/test_datatypes.py +++ b/tests/tests_integration/test_datatypes.py @@ -1,4 +1,5 @@ import os +import random import pytest import test_utils @@ -61,8 +62,6 @@ def test_transfer_datatypes( """ subs, sessions = test_utils.get_default_sub_sessions_to_test() - # Unfortunately on Windows we are encountering 'The command line is too long' - # and so cannot test against all datatypes here. narrow_datatypes = canonical_configs.quick_get_narrow_datatypes() datatypes_used = self.get_narrow_only_datatypes_used(used=False) From f9b999a1bb79c6ea120852e0b383f3dba85a580d Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Thu, 8 May 2025 18:23:46 +0100 Subject: [PATCH 03/10] Fix tooltips. --- datashuttle/configs/canonical_configs.py | 62 +++++++++++++++++++++--- datashuttle/datashuttle_class.py | 11 ++--- datashuttle/tui/screens/datatypes.py | 4 +- 3 files changed, 62 insertions(+), 15 deletions(-) diff --git a/datashuttle/configs/canonical_configs.py b/datashuttle/configs/canonical_configs.py index 374d945cc..b22bc09bf 100644 --- a/datashuttle/configs/canonical_configs.py +++ b/datashuttle/configs/canonical_configs.py @@ -351,7 +351,7 @@ def quick_get_narrow_datatypes(): return flat_narrow_datatypes -def in_place_update_settings_for_narrow_datatype(settings: dict): +def in_place_update_narrow_datatypes_if_required(user_settings: dict): """ In versions < v0.6.0, only 'broad' datatypes were implemented and available in the TUI. Since, 'narrow' datatypes are introduced @@ -360,8 +360,39 @@ def in_place_update_settings_for_narrow_datatype(settings: dict): This function converts the old format to the new format so that all broad datatype settings (on / off) are maintained in - then new version. + then new version. It does this by copying the full default + parameters and overwriting them with the available user-set + defaults. This is the best approach, as it maintains the + order of the datatypes (otherwise, inserting non-existing + datatypes into the user datatype dict results in the wrong order). + """ + has_narrow_datatypes = isinstance( + user_settings["tui"]["create_checkboxes_on"]["behav"], dict + ) # added 'narrow datatype' v0.6.0 with major refactor to dict + + all_narrow_datatypes = quick_get_narrow_datatypes() + + is_not_missing_narrow_datatypes = all( + [ + dtype in all_narrow_datatypes + for dtype in user_settings["tui"]["create_checkboxes_on"] + ] + ) + + if is_not_missing_narrow_datatypes: + assert all( + [ + dtype in all_narrow_datatypes + for dtype in user_settings["tui"]["transfer_checkboxes_on"] + ] + ) + + if has_narrow_datatypes and is_not_missing_narrow_datatypes: + return + + # Copy TUI defaults that include narrow-datatype defaults + canonical_tui_configs = get_tui_config_defaults() new_create_checkbox_configs = copy.deepcopy( @@ -371,18 +402,35 @@ def in_place_update_settings_for_narrow_datatype(settings: dict): canonical_tui_configs["tui"]["transfer_checkboxes_on"] ) + # Copy the pre-existing settings for broad datatypes for key in ["behav", "ephys", "funcimg", "anat"]: - new_create_checkbox_configs[key]["on"] = settings["tui"][ + new_create_checkbox_configs[key]["on"] = user_settings["tui"][ "create_checkboxes_on" ][key] - new_transfer_checkbox_configs[key]["on"] = settings["tui"][ + new_transfer_checkbox_configs[key]["on"] = user_settings["tui"][ "transfer_checkboxes_on" ][key] + # Copy the pre-existing settings for the transfer checkboxes for key in ["all", "all_datatype", "all_non_datatype"]: - new_transfer_checkbox_configs[key]["on"] = settings["tui"][ + new_transfer_checkbox_configs[key]["on"] = user_settings["tui"][ "transfer_checkboxes_on" ][key] - settings["tui"]["create_checkboxes_on"] = new_create_checkbox_configs - settings["tui"]["transfer_checkboxes_on"] = new_transfer_checkbox_configs + for narrow_datatype in all_narrow_datatypes: + if ( + narrow_datatype + not in user_settings["tui"]["create_checkboxes_on"].keys() + ): + user_settings["tui"][ + "create_checkboxes_on" + ] = new_create_checkbox_configs + + for narrow_datatype in all_narrow_datatypes: + if ( + narrow_datatype + not in user_settings["tui"]["transfer_checkboxes_on"].keys() + ): + user_settings["tui"][ + "transfer_checkboxes_on" + ] = new_transfer_checkbox_configs diff --git a/datashuttle/datashuttle_class.py b/datashuttle/datashuttle_class.py index 9322323fa..48b8fcab0 100644 --- a/datashuttle/datashuttle_class.py +++ b/datashuttle/datashuttle_class.py @@ -1551,13 +1551,10 @@ def _update_settings_with_new_canonical_keys(self, settings: Dict): if key not in settings["tui"]: settings["tui"][key] = canonical_tui_configs["tui"][key] - # Handle conversion to 'narrow datatype' v0.6.0 - if not isinstance( - settings["tui"]["create_checkboxes_on"]["behav"], dict - ): - canonical_configs.in_place_update_settings_for_narrow_datatype( - settings - ) + # Handle updating with narrow datatypes + canonical_configs.in_place_update_narrow_datatypes_if_required( + settings + ) def _check_top_level_folder(self, top_level_folder): """ diff --git a/datashuttle/tui/screens/datatypes.py b/datashuttle/tui/screens/datatypes.py index 790cca553..df184d4e6 100644 --- a/datashuttle/tui/screens/datatypes.py +++ b/datashuttle/tui/screens/datatypes.py @@ -33,12 +33,14 @@ "behav": "behaviour", "funcimg": "functional imaging", "anat": "anatomy", + "motion": "motion tracking", "ecephys": "extracellular electrophysiology", "icephys": "intracellular electrophysiology", + "emg": "electromyography", "cscope": "head-mounted widefield macroscope", "f2pe": "functional 2-photon excitation imaging", "fmri": "functional magnetic resonance imaging", - "fusi": "functional ultra-sound imaging", + "fusi": "functional ultrasound imaging", "2pe": "2-photon excitation microscopy", "bf": "bright-field microscopy", "cars": "coherent anti-Stokes Raman spectroscopy", From 8573a44ca71e7855bd1a954a744dc7332b2bf81d Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Fri, 6 Jun 2025 13:18:35 +0100 Subject: [PATCH 04/10] Fix tests, update backwards compatability function. --- datashuttle/configs/canonical_configs.py | 107 +++++----- .../old_version_configs/README.md | 7 + .../old_version_configs/v0.5.3/config.yaml | 5 + .../v0.5.3/persistent_settings.yaml | 25 +++ .../old_version_configs/v0.6.0/config.yaml | 5 + .../v0.6.0/persistent_settings.yaml | 191 ++++++++++++++++++ .../test_backwards_compatibility.py | 137 +++++++++++++ 7 files changed, 429 insertions(+), 48 deletions(-) create mode 100644 tests/tests_regression/old_version_configs/README.md create mode 100644 tests/tests_regression/old_version_configs/v0.5.3/config.yaml create mode 100644 tests/tests_regression/old_version_configs/v0.5.3/persistent_settings.yaml create mode 100644 tests/tests_regression/old_version_configs/v0.6.0/config.yaml create mode 100644 tests/tests_regression/old_version_configs/v0.6.0/persistent_settings.yaml create mode 100644 tests/tests_regression/test_backwards_compatibility.py diff --git a/datashuttle/configs/canonical_configs.py b/datashuttle/configs/canonical_configs.py index b22bc09bf..91e8d3bc4 100644 --- a/datashuttle/configs/canonical_configs.py +++ b/datashuttle/configs/canonical_configs.py @@ -22,7 +22,6 @@ if TYPE_CHECKING: from datashuttle.configs.config_class import Configs -import copy from pathlib import Path import typeguard @@ -367,70 +366,82 @@ def in_place_update_narrow_datatypes_if_required(user_settings: dict): datatypes into the user datatype dict results in the wrong order). """ + # Find out what is included in the loaded config file, + # that determines its version + has_narrow_datatypes = isinstance( user_settings["tui"]["create_checkboxes_on"]["behav"], dict ) # added 'narrow datatype' v0.6.0 with major refactor to dict all_narrow_datatypes = quick_get_narrow_datatypes() - is_not_missing_narrow_datatypes = all( + is_not_missing_any_narrow_datatypes = all( [ - dtype in all_narrow_datatypes - for dtype in user_settings["tui"]["create_checkboxes_on"] + dtype in user_settings["tui"]["create_checkboxes_on"] + for dtype in all_narrow_datatypes ] ) - if is_not_missing_narrow_datatypes: + if is_not_missing_any_narrow_datatypes: assert all( [ - dtype in all_narrow_datatypes - for dtype in user_settings["tui"]["transfer_checkboxes_on"] + dtype in user_settings["tui"]["transfer_checkboxes_on"] + for dtype in all_narrow_datatypes ] - ) + ), "Somehow there are datatypes missing in `transfer_checkboxes_on` but not `create_checkboxes_on`" - if has_narrow_datatypes and is_not_missing_narrow_datatypes: + if has_narrow_datatypes and is_not_missing_any_narrow_datatypes: return - # Copy TUI defaults that include narrow-datatype defaults - + # Make a dictionary of the canonical configs to fill in with whatever + # user data exists. This ensures the order of the keys is always the same. canonical_tui_configs = get_tui_config_defaults() - new_create_checkbox_configs = copy.deepcopy( - canonical_tui_configs["tui"]["create_checkboxes_on"] - ) - new_transfer_checkbox_configs = copy.deepcopy( - canonical_tui_configs["tui"]["transfer_checkboxes_on"] - ) - - # Copy the pre-existing settings for broad datatypes - for key in ["behav", "ephys", "funcimg", "anat"]: - new_create_checkbox_configs[key]["on"] = user_settings["tui"][ - "create_checkboxes_on" - ][key] - new_transfer_checkbox_configs[key]["on"] = user_settings["tui"][ - "transfer_checkboxes_on" - ][key] + new_checkbox_configs = { + "create_checkboxes_on": ( + canonical_tui_configs["tui"]["create_checkboxes_on"] + ), + "transfer_checkboxes_on": ( + canonical_tui_configs["tui"]["transfer_checkboxes_on"] + ), + } - # Copy the pre-existing settings for the transfer checkboxes + # Copy the pre-existing settings unique to the transfer checkboxes for key in ["all", "all_datatype", "all_non_datatype"]: - new_transfer_checkbox_configs[key]["on"] = user_settings["tui"][ - "transfer_checkboxes_on" - ][key] - - for narrow_datatype in all_narrow_datatypes: - if ( - narrow_datatype - not in user_settings["tui"]["create_checkboxes_on"].keys() - ): - user_settings["tui"][ - "create_checkboxes_on" - ] = new_create_checkbox_configs - - for narrow_datatype in all_narrow_datatypes: - if ( - narrow_datatype - not in user_settings["tui"]["transfer_checkboxes_on"].keys() - ): - user_settings["tui"][ - "transfer_checkboxes_on" - ] = new_transfer_checkbox_configs + if has_narrow_datatypes: + new_checkbox_configs["transfer_checkboxes_on"][key] = ( + user_settings["tui"]["transfer_checkboxes_on"][key] + ) + else: + new_checkbox_configs["transfer_checkboxes_on"][key]["on"] = ( + user_settings["tui"]["transfer_checkboxes_on"][key] + ) + + # Copy any datatype information that exists. Broad datatypes will all be there + # but some narrow datatypes might be missing. + for checkbox_type in ["create_checkboxes_on", "transfer_checkboxes_on"]: + + datatypes_that_user_has = list( + user_settings["tui"][checkbox_type].keys() + ) + + for dtype in get_datatypes(): + + if dtype in datatypes_that_user_has: + + if has_narrow_datatypes: + new_checkbox_configs[checkbox_type][dtype] = user_settings[ + "tui" + ][checkbox_type][dtype] + else: + # in versions < 0.6.0 the datatype settings was only a bool + # indicating whether the checkbox is on or not. New versions + # are a dictionary indicating if the checkbox is on ("on") + # and displayed ("displayed"). + new_checkbox_configs[checkbox_type][dtype]["on"] = ( + user_settings["tui"][checkbox_type][dtype] + ) + + user_settings["tui"][checkbox_type] = new_checkbox_configs[ + checkbox_type + ] diff --git a/tests/tests_regression/old_version_configs/README.md b/tests/tests_regression/old_version_configs/README.md new file mode 100644 index 000000000..d92d6d46d --- /dev/null +++ b/tests/tests_regression/old_version_configs/README.md @@ -0,0 +1,7 @@ +This config and persistent settings are copied from a project created in the GUI +for the purposes of this regression test. They key information is: + +1) The project is called "test_project" +2) All create and transfer checkboxes are not displayed, except for "f2pe". +This is so we can check the persistent settings are updated as expected. +3) The diff --git a/tests/tests_regression/old_version_configs/v0.5.3/config.yaml b/tests/tests_regression/old_version_configs/v0.5.3/config.yaml new file mode 100644 index 000000000..7148d7c9d --- /dev/null +++ b/tests/tests_regression/old_version_configs/v0.5.3/config.yaml @@ -0,0 +1,5 @@ +local_path: old_ver +central_path: old_ver +connection_method: local_filesystem +central_host_id: null +central_host_username: null diff --git a/tests/tests_regression/old_version_configs/v0.5.3/persistent_settings.yaml b/tests/tests_regression/old_version_configs/v0.5.3/persistent_settings.yaml new file mode 100644 index 000000000..81d6d6e17 --- /dev/null +++ b/tests/tests_regression/old_version_configs/v0.5.3/persistent_settings.yaml @@ -0,0 +1,25 @@ +tui: + create_checkboxes_on: + behav: false + ephys: false + funcimg: false + anat: false + transfer_checkboxes_on: + behav: false + ephys: false + funcimg: false + anat: false + all: false + all_datatype: false + all_non_datatype: false + top_level_folder_select: + create_tab: rawdata + toplevel_transfer: rawdata + custom_transfer: rawdata + bypass_validation: false + overwrite_existing_files: never + dry_run: false +name_templates: + 'on': false + sub: null + ses: null diff --git a/tests/tests_regression/old_version_configs/v0.6.0/config.yaml b/tests/tests_regression/old_version_configs/v0.6.0/config.yaml new file mode 100644 index 000000000..7148d7c9d --- /dev/null +++ b/tests/tests_regression/old_version_configs/v0.6.0/config.yaml @@ -0,0 +1,5 @@ +local_path: old_ver +central_path: old_ver +connection_method: local_filesystem +central_host_id: null +central_host_username: null diff --git a/tests/tests_regression/old_version_configs/v0.6.0/persistent_settings.yaml b/tests/tests_regression/old_version_configs/v0.6.0/persistent_settings.yaml new file mode 100644 index 000000000..165dd4cdb --- /dev/null +++ b/tests/tests_regression/old_version_configs/v0.6.0/persistent_settings.yaml @@ -0,0 +1,191 @@ +tui: + create_checkboxes_on: + ephys: + 'on': false + displayed: false + behav: + 'on': false + displayed: false + funcimg: + 'on': false + displayed: false + anat: + 'on': false + displayed: false + ecephys: + 'on': false + displayed: false + icephys: + 'on': false + displayed: false + cscope: + 'on': false + displayed: false + f2pe: + 'on': false + displayed: true + fmri: + 'on': false + displayed: false + fusi: + 'on': false + displayed: false + 2pe: + 'on': false + displayed: false + bf: + 'on': false + displayed: false + cars: + 'on': false + displayed: false + conf: + 'on': false + displayed: false + dic: + 'on': false + displayed: false + df: + 'on': false + displayed: false + fluo: + 'on': false + displayed: false + mpe: + 'on': false + displayed: false + nlo: + 'on': false + displayed: false + oct: + 'on': false + displayed: false + pc: + 'on': false + displayed: false + pli: + 'on': false + displayed: false + sem: + 'on': false + displayed: false + spim: + 'on': false + displayed: false + sr: + 'on': false + displayed: false + tem: + 'on': false + displayed: false + uct: + 'on': false + displayed: false + mri: + 'on': false + displayed: false + transfer_checkboxes_on: + all: + 'on': false + displayed: false + all_datatype: + 'on': false + displayed: false + all_non_datatype: + 'on': false + displayed: false + ephys: + 'on': false + displayed: false + behav: + 'on': false + displayed: false + funcimg: + 'on': false + displayed: false + anat: + 'on': false + displayed: false + ecephys: + 'on': false + displayed: false + icephys: + 'on': false + displayed: false + cscope: + 'on': false + displayed: false + f2pe: + 'on': false + displayed: true + fmri: + 'on': false + displayed: false + fusi: + 'on': false + displayed: false + 2pe: + 'on': false + displayed: false + bf: + 'on': false + displayed: false + cars: + 'on': false + displayed: false + conf: + 'on': false + displayed: false + dic: + 'on': false + displayed: false + df: + 'on': false + displayed: false + fluo: + 'on': false + displayed: false + mpe: + 'on': false + displayed: false + nlo: + 'on': false + displayed: false + oct: + 'on': false + displayed: false + pc: + 'on': false + displayed: false + pli: + 'on': false + displayed: false + sem: + 'on': false + displayed: false + spim: + 'on': false + displayed: false + sr: + 'on': false + displayed: false + tem: + 'on': false + displayed: false + uct: + 'on': false + displayed: false + mri: + 'on': false + displayed: false + top_level_folder_select: + create_tab: rawdata + toplevel_transfer: rawdata + custom_transfer: rawdata + bypass_validation: false + overwrite_existing_files: never + dry_run: false +name_templates: + 'on': false + sub: null + ses: null diff --git a/tests/tests_regression/test_backwards_compatibility.py b/tests/tests_regression/test_backwards_compatibility.py new file mode 100644 index 000000000..8615694b2 --- /dev/null +++ b/tests/tests_regression/test_backwards_compatibility.py @@ -0,0 +1,137 @@ +import shutil +from pathlib import Path + +from datashuttle import DataShuttle + + +class TestBackwardsCompatibility: + + def test_v0_6_0(self): + """ + v0.6.0 is the first version with narrow datatypes, and the checkboxes was refactored to + be a {"on": bool, "displayed": bool} dict rather than a bool indicating whether the checkbox is on. + However, this version is missing narrow datatypes added later (e.g. "motion"). + In the test file, all 'displayed' are turned off except f2pe. + """ + reloaded_ver_configs, reloaded_ver_persistent_settings = ( + self.load_and_check_old_version_yamls("v0.6.0") + ) + + assert reloaded_ver_configs["local_path"] == Path("old_ver") + + reloaded_create_checkboxes = reloaded_ver_persistent_settings["tui"][ + "create_checkboxes_on" + ] + transfer_checkboxes = reloaded_ver_persistent_settings["tui"][ + "transfer_checkboxes_on" + ] + + assert reloaded_create_checkboxes["ephys"]["displayed"] is False + assert reloaded_create_checkboxes["motion"]["displayed"] is False + assert reloaded_create_checkboxes["f2pe"]["displayed"] is True + + assert transfer_checkboxes["ephys"]["displayed"] is False + assert transfer_checkboxes["all"]["displayed"] is False + assert transfer_checkboxes["motion"]["displayed"] is False + assert transfer_checkboxes["f2pe"]["displayed"] is True + + def test_v0_5_3(self): + """ + This version did not have narrow datatypes, and the persistent checkbox setting was only a + bool. Therefore, the "displayed" uses the canonical defaults (because they don't exist in the file yet). + """ + reloaded_ver_configs, reloaded_ver_persistent_settings = ( + self.load_and_check_old_version_yamls("v0.5.3") + ) + + assert reloaded_ver_configs["local_path"] == Path("old_ver") + + reloaded_create_checkboxes = reloaded_ver_persistent_settings["tui"][ + "create_checkboxes_on" + ] + transfer_checkboxes = reloaded_ver_persistent_settings["tui"][ + "transfer_checkboxes_on" + ] + + assert reloaded_create_checkboxes["ephys"]["displayed"] is True + assert reloaded_create_checkboxes["motion"]["displayed"] is False + assert reloaded_create_checkboxes["f2pe"]["displayed"] is False + + assert transfer_checkboxes["ephys"]["displayed"] is True + assert transfer_checkboxes["all"]["displayed"] is True + assert transfer_checkboxes["motion"]["displayed"] is False + assert transfer_checkboxes["f2pe"]["displayed"] is False + + def load_and_check_old_version_yamls(self, datashuttle_version): + """ + Load an old config file in the current datashuttle version, + and check that the new-version ('canonical') configs + and persistent settings match the structure of the + files loaded from the old datashuttle version. + """ + project = DataShuttle("test_project") + + # Set up paths and clear any existing config files for this project + old_version_path = ( + Path(__file__).parent / "old_version_configs" / datashuttle_version + ) + + config_file_path = project._config_path + config_path = config_file_path.parent + + (config_file_path).unlink(missing_ok=True) + (config_path / "persistent_settings.yaml").unlink(missing_ok=True) + + # In the current version of datashuttle, get the settings. These are + # thus correct for the most recent datashuttle version. + project = DataShuttle("test_project") + project.make_config_file("cur_ver", "cur_ver", "local_filesystem") + + current_ver_configs = project.get_configs() + current_ver_persistent_settings = project._load_persistent_settings() + + # Copy from the test paths the old version settings to the + # project folder. Now when datashuttle loads, it will load from + # these old version files. Check that these match the current version + # files in structure. + shutil.copy(old_version_path / "config.yaml", config_path) + shutil.copy(old_version_path / "persistent_settings.yaml", config_path) + + project = DataShuttle("test_project") + + reloaded_ver_configs = project.get_configs() + reloaded_ver_persistent_settings = project._load_persistent_settings() + + self.recursive_test_dictionary( + current_ver_configs, reloaded_ver_configs + ) + self.recursive_test_dictionary( + current_ver_persistent_settings, reloaded_ver_persistent_settings + ) + + return reloaded_ver_configs, reloaded_ver_persistent_settings + + def recursive_test_dictionary(self, dict_canonical, dict_to_test): + """ + A dictionary to check all keys in a nested dictionary + match and all value types are the same. + """ + keys_canonical = list(dict_canonical.keys()) + keys_to_test = list(dict_to_test.keys()) + + assert keys_canonical == keys_to_test, ( + f"Keys are either missing or in the incorrect order:" + f"\nkeys_canonical:\n {keys_canonical}" + f"\nkeys_to_test:\n {keys_to_test}" + ) + + for key in dict_canonical.keys(): + + if isinstance(dict_canonical[key], dict): + self.recursive_test_dictionary( + dict_canonical[key], dict_to_test[key] + ) + else: + assert isinstance( + dict_to_test[key], type(dict_canonical[key]) + ), f"key: {key} is not the correct type" From 783680b44d2cd0b805eac8067c3e278e87f69e4c Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 6 Jun 2025 12:37:09 +0000 Subject: [PATCH 05/10] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/tests_integration/test_datatypes.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/tests_integration/test_datatypes.py b/tests/tests_integration/test_datatypes.py index 67545917f..840fc26ee 100644 --- a/tests/tests_integration/test_datatypes.py +++ b/tests/tests_integration/test_datatypes.py @@ -1,5 +1,4 @@ import os -import random import pytest import test_utils From f6eecd5fd6c6dd426b2d6d028f4f62e2d6b97df7 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Thu, 12 Jun 2025 20:58:17 +0100 Subject: [PATCH 06/10] Loop through checkboxes for backwards test. --- .../test_backwards_compatibility.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/tests/tests_regression/test_backwards_compatibility.py b/tests/tests_regression/test_backwards_compatibility.py index 8615694b2..d4b5f505c 100644 --- a/tests/tests_regression/test_backwards_compatibility.py +++ b/tests/tests_regression/test_backwards_compatibility.py @@ -26,14 +26,13 @@ def test_v0_6_0(self): "transfer_checkboxes_on" ] - assert reloaded_create_checkboxes["ephys"]["displayed"] is False - assert reloaded_create_checkboxes["motion"]["displayed"] is False - assert reloaded_create_checkboxes["f2pe"]["displayed"] is True + for key in reloaded_create_checkboxes.keys(): + assert reloaded_create_checkboxes[key]["displayed"] is ( + key == "f2pe" + ) - assert transfer_checkboxes["ephys"]["displayed"] is False - assert transfer_checkboxes["all"]["displayed"] is False - assert transfer_checkboxes["motion"]["displayed"] is False - assert transfer_checkboxes["f2pe"]["displayed"] is True + for key in transfer_checkboxes.keys(): + assert transfer_checkboxes[key]["displayed"] is (key == "f2pe") def test_v0_5_3(self): """ From f29a8d8cb74bcdf0e1538c33ee40604633670a01 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Thu, 12 Jun 2025 21:45:05 +0100 Subject: [PATCH 07/10] Try removing possibly unused cwd in tests. --- tests/test_utils.py | 7 ++----- tests/tests_integration/base.py | 4 +--- tests/tests_integration/test_logging.py | 4 ++-- tests/tests_integration/test_ssh_file_transfer.py | 4 ++-- tests/tests_integration/test_ssh_setup.py | 6 ++---- tests/tests_regression/test_backwards_compatibility.py | 9 +++++++++ 6 files changed, 18 insertions(+), 16 deletions(-) diff --git a/tests/test_utils.py b/tests/test_utils.py index ad908160e..149153894 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -90,10 +90,9 @@ def glob_basenames(search_path, recursive=False, exclude=None): def teardown_project( - cwd, project + project, ): # 99% sure these are unnecessary with pytest tmp_path but keep until SSH testing. """""" - os.chdir(cwd) delete_all_folders_in_project_path(project, "central") delete_all_folders_in_project_path(project, "local") delete_project_if_it_exists(project.project_name) @@ -151,9 +150,7 @@ def setup_project_fixture(tmp_path, test_project_name, project_type="full"): local_path=make_test_path(tmp_path, "local", test_project_name) ) - cwd = os.getcwd() - - return project, cwd + return project def make_test_path(base_path, local_or_central, test_project_name): diff --git a/tests/tests_integration/base.py b/tests/tests_integration/base.py index b04816cba..bee36b019 100644 --- a/tests/tests_integration/base.py +++ b/tests/tests_integration/base.py @@ -1,4 +1,3 @@ -import os import warnings import pytest @@ -58,9 +57,8 @@ def project(self, tmp_path, request): else: raise ValueError("`parametrized value must be 'full' or 'local'") - cwd = os.getcwd() yield project - test_utils.teardown_project(cwd, project) + test_utils.teardown_project(project) @pytest.fixture(scope="function") def clean_project_name(self): diff --git a/tests/tests_integration/test_logging.py b/tests/tests_integration/test_logging.py index 4e4c3393b..3cf2d733e 100644 --- a/tests/tests_integration/test_logging.py +++ b/tests/tests_integration/test_logging.py @@ -118,7 +118,7 @@ def project(self, tmp_path, clean_project_name, request): """ project_type = getattr(request, "param", "full") - project, cwd = test_utils.setup_project_fixture( + project = test_utils.setup_project_fixture( tmp_path, clean_project_name, project_type ) @@ -128,7 +128,7 @@ def project(self, tmp_path, clean_project_name, request): yield project - test_utils.teardown_project(cwd, project) + test_utils.teardown_project(project) test_utils.set_datashuttle_loggers(disable=True) # ---------------------------------------------------------------------------------------------------------- diff --git a/tests/tests_integration/test_ssh_file_transfer.py b/tests/tests_integration/test_ssh_file_transfer.py index a94a5b05a..393de076c 100644 --- a/tests/tests_integration/test_ssh_file_transfer.py +++ b/tests/tests_integration/test_ssh_file_transfer.py @@ -80,7 +80,7 @@ def pathtable_and_project(self, request, tmpdir_factory): central_path = base_path test_project_name = "test_file_conflicts" - project, cwd = test_utils.setup_project_fixture( + project = test_utils.setup_project_fixture( base_path, test_project_name ) @@ -104,7 +104,7 @@ def pathtable_and_project(self, request, tmpdir_factory): yield [pathtable, project] - test_utils.teardown_project(cwd, project) + test_utils.teardown_project(project) if testing_ssh: for result in glob.glob(ssh_config.FILESYSTEM_PATH): diff --git a/tests/tests_integration/test_ssh_setup.py b/tests/tests_integration/test_ssh_setup.py index 45ad0f51d..752985a67 100644 --- a/tests/tests_integration/test_ssh_setup.py +++ b/tests/tests_integration/test_ssh_setup.py @@ -23,9 +23,7 @@ def project(test, tmp_path): tmp_path = tmp_path / "test with space" test_project_name = "test_ssh" - project, cwd = test_utils.setup_project_fixture( - tmp_path, test_project_name - ) + project = test_utils.setup_project_fixture(tmp_path, test_project_name) ssh_test_utils.setup_project_for_ssh( project, @@ -35,7 +33,7 @@ def project(test, tmp_path): ) yield project - test_utils.teardown_project(cwd, project) + test_utils.teardown_project(project) # ----------------------------------------------------------------- # Test Setup SSH Connection diff --git a/tests/tests_regression/test_backwards_compatibility.py b/tests/tests_regression/test_backwards_compatibility.py index d4b5f505c..234fc5944 100644 --- a/tests/tests_regression/test_backwards_compatibility.py +++ b/tests/tests_regression/test_backwards_compatibility.py @@ -1,11 +1,20 @@ import shutil from pathlib import Path +import pytest + from datashuttle import DataShuttle class TestBackwardsCompatibility: + @pytest.fixture(scope="function") + def project(self): + """ """ + project = DataShuttle("test_project") + + yield project + def test_v0_6_0(self): """ v0.6.0 is the first version with narrow datatypes, and the checkboxes was refactored to From ec2f78665bf8377d206d9db7c11ce010f65e2b71 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Thu, 12 Jun 2025 22:36:31 +0100 Subject: [PATCH 08/10] Add fixture to tear down configs and make sure project init folders are not written. --- tests/test_utils.py | 4 ++- .../test_backwards_compatibility.py | 35 +++++++++++++------ 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/tests/test_utils.py b/tests/test_utils.py index 149153894..66bd6d19b 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -108,7 +108,9 @@ def delete_all_folders_in_project_path(project, local_or_central): """""" folder = f"{local_or_central}_path" - if folder == "central_path" and project.cfg[folder] is None: + if project.cfg is None or ( + folder == "central_path" and project.cfg[folder] is None + ): return ds_logger.close_log_filehandler() diff --git a/tests/tests_regression/test_backwards_compatibility.py b/tests/tests_regression/test_backwards_compatibility.py index 234fc5944..610a63467 100644 --- a/tests/tests_regression/test_backwards_compatibility.py +++ b/tests/tests_regression/test_backwards_compatibility.py @@ -1,21 +1,32 @@ +import os import shutil from pathlib import Path import pytest +import test_utils from datashuttle import DataShuttle +TEST_PROJECT_NAME = "test_project" + class TestBackwardsCompatibility: @pytest.fixture(scope="function") def project(self): - """ """ - project = DataShuttle("test_project") + """ + Delete the project configs if they exist, + and tear down after the test has run. + """ + test_utils.delete_project_if_it_exists(TEST_PROJECT_NAME) + + project = DataShuttle(TEST_PROJECT_NAME) yield project - def test_v0_6_0(self): + test_utils.delete_project_if_it_exists(TEST_PROJECT_NAME) + + def test_v0_6_0(self, project, tmp_path): """ v0.6.0 is the first version with narrow datatypes, and the checkboxes was refactored to be a {"on": bool, "displayed": bool} dict rather than a bool indicating whether the checkbox is on. @@ -23,7 +34,7 @@ def test_v0_6_0(self): In the test file, all 'displayed' are turned off except f2pe. """ reloaded_ver_configs, reloaded_ver_persistent_settings = ( - self.load_and_check_old_version_yamls("v0.6.0") + self.load_and_check_old_version_yamls(project, tmp_path, "v0.6.0") ) assert reloaded_ver_configs["local_path"] == Path("old_ver") @@ -43,13 +54,13 @@ def test_v0_6_0(self): for key in transfer_checkboxes.keys(): assert transfer_checkboxes[key]["displayed"] is (key == "f2pe") - def test_v0_5_3(self): + def test_v0_5_3(self, project, tmp_path): """ This version did not have narrow datatypes, and the persistent checkbox setting was only a bool. Therefore, the "displayed" uses the canonical defaults (because they don't exist in the file yet). """ reloaded_ver_configs, reloaded_ver_persistent_settings = ( - self.load_and_check_old_version_yamls("v0.5.3") + self.load_and_check_old_version_yamls(project, tmp_path, "v0.5.3") ) assert reloaded_ver_configs["local_path"] == Path("old_ver") @@ -70,14 +81,18 @@ def test_v0_5_3(self): assert transfer_checkboxes["motion"]["displayed"] is False assert transfer_checkboxes["f2pe"]["displayed"] is False - def load_and_check_old_version_yamls(self, datashuttle_version): + def load_and_check_old_version_yamls( + self, project, tmp_path, datashuttle_version + ): """ Load an old config file in the current datashuttle version, and check that the new-version ('canonical') configs and persistent settings match the structure of the files loaded from the old datashuttle version. """ - project = DataShuttle("test_project") + # Switch dir so folders created in `DataShuttle` init do + # not pollute the users test drive. + os.chdir(tmp_path) # Set up paths and clear any existing config files for this project old_version_path = ( @@ -92,7 +107,7 @@ def load_and_check_old_version_yamls(self, datashuttle_version): # In the current version of datashuttle, get the settings. These are # thus correct for the most recent datashuttle version. - project = DataShuttle("test_project") + project = DataShuttle(TEST_PROJECT_NAME) project.make_config_file("cur_ver", "cur_ver", "local_filesystem") current_ver_configs = project.get_configs() @@ -105,7 +120,7 @@ def load_and_check_old_version_yamls(self, datashuttle_version): shutil.copy(old_version_path / "config.yaml", config_path) shutil.copy(old_version_path / "persistent_settings.yaml", config_path) - project = DataShuttle("test_project") + project = DataShuttle(TEST_PROJECT_NAME) reloaded_ver_configs = project.get_configs() reloaded_ver_persistent_settings = project._load_persistent_settings() From 7173bd8600303adf2178153c3b6f79740d38d286 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Thu, 19 Jun 2025 11:57:59 +0100 Subject: [PATCH 09/10] Remove now unecessary delete lines. --- tests/tests_regression/test_backwards_compatibility.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/tests_regression/test_backwards_compatibility.py b/tests/tests_regression/test_backwards_compatibility.py index 610a63467..1d977dbbb 100644 --- a/tests/tests_regression/test_backwards_compatibility.py +++ b/tests/tests_regression/test_backwards_compatibility.py @@ -102,8 +102,8 @@ def load_and_check_old_version_yamls( config_file_path = project._config_path config_path = config_file_path.parent - (config_file_path).unlink(missing_ok=True) - (config_path / "persistent_settings.yaml").unlink(missing_ok=True) + # (config_file_path).unlink(missing_ok=True) + # (config_path / "persistent_settings.yaml").unlink(missing_ok=True) # In the current version of datashuttle, get the settings. These are # thus correct for the most recent datashuttle version. From bbb569e0f280a1cb32025b6a02c531e3256486a1 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Thu, 19 Jun 2025 12:33:43 +0100 Subject: [PATCH 10/10] Actually remove the lines not comment. --- tests/tests_regression/test_backwards_compatibility.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/tests_regression/test_backwards_compatibility.py b/tests/tests_regression/test_backwards_compatibility.py index 1d977dbbb..7e243992a 100644 --- a/tests/tests_regression/test_backwards_compatibility.py +++ b/tests/tests_regression/test_backwards_compatibility.py @@ -102,9 +102,6 @@ def load_and_check_old_version_yamls( config_file_path = project._config_path config_path = config_file_path.parent - # (config_file_path).unlink(missing_ok=True) - # (config_path / "persistent_settings.yaml").unlink(missing_ok=True) - # In the current version of datashuttle, get the settings. These are # thus correct for the most recent datashuttle version. project = DataShuttle(TEST_PROJECT_NAME)