diff --git a/datashuttle/configs/canonical_configs.py b/datashuttle/configs/canonical_configs.py index b65ad6c66..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 @@ -310,7 +309,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", @@ -350,7 +350,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 @@ -359,29 +359,89 @@ 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). + """ - canonical_tui_configs = get_tui_config_defaults() + # Find out what is included in the loaded config file, + # that determines its version - 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"] + 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_any_narrow_datatypes = all( + [ + dtype in user_settings["tui"]["create_checkboxes_on"] + for dtype in all_narrow_datatypes + ] ) - for key in ["behav", "ephys", "funcimg", "anat"]: - new_create_checkbox_configs[key]["on"] = settings["tui"][ - "create_checkboxes_on" - ][key] - new_transfer_checkbox_configs[key]["on"] = settings["tui"][ - "transfer_checkboxes_on" - ][key] + if is_not_missing_any_narrow_datatypes: + assert all( + [ + 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_any_narrow_datatypes: + return + + # 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_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 unique to the transfer checkboxes for key in ["all", "all_datatype", "all_non_datatype"]: - new_transfer_checkbox_configs[key]["on"] = settings["tui"][ - "transfer_checkboxes_on" - ][key] + 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() + ) - settings["tui"]["create_checkboxes_on"] = new_create_checkbox_configs - settings["tui"]["transfer_checkboxes_on"] = new_transfer_checkbox_configs + 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/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", diff --git a/tests/test_utils.py b/tests/test_utils.py index ad908160e..66bd6d19b 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) @@ -109,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() @@ -151,9 +152,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_datatypes.py b/tests/tests_integration/test_datatypes.py index 1e1776e41..840fc26ee 100644 --- a/tests/tests_integration/test_datatypes.py +++ b/tests/tests_integration/test_datatypes.py @@ -61,8 +61,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) 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/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..7e243992a --- /dev/null +++ b/tests/tests_regression/test_backwards_compatibility.py @@ -0,0 +1,157 @@ +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): + """ + 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 + + 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. + 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(project, tmp_path, "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" + ] + + for key in reloaded_create_checkboxes.keys(): + assert reloaded_create_checkboxes[key]["displayed"] is ( + key == "f2pe" + ) + + for key in transfer_checkboxes.keys(): + assert transfer_checkboxes[key]["displayed"] is (key == "f2pe") + + 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(project, tmp_path, "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, 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. + """ + # 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 = ( + Path(__file__).parent / "old_version_configs" / datashuttle_version + ) + + config_file_path = project._config_path + config_path = config_file_path.parent + + # In the current version of datashuttle, get the settings. These are + # thus correct for the most recent datashuttle version. + project = DataShuttle(TEST_PROJECT_NAME) + 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_NAME) + + 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"