From 2225f6e59683da50518eb73a4787741bb47bf76d Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Wed, 7 Jun 2023 20:24:58 +0100 Subject: [PATCH] Add tests to check warning raised for inconsistent sub or ses leading zeros. --- datashuttle/datashuttle.py | 14 ++-- datashuttle/utils/formatting.py | 12 ++-- tests/tests_integration/test_formatting.py | 76 ++++++++++++++++++++++ tests/tests_unit/test_unit.py | 10 +++ 4 files changed, 101 insertions(+), 11 deletions(-) diff --git a/datashuttle/datashuttle.py b/datashuttle/datashuttle.py index 50f214450..f6623863e 100644 --- a/datashuttle/datashuttle.py +++ b/datashuttle/datashuttle.py @@ -353,7 +353,7 @@ def upload( if init_log: self._start_log("upload", local_vars=locals()) - self.show_top_level_folder() + self._show_pre_transfer_messages() TransferData( self.cfg, @@ -389,7 +389,7 @@ def download( if init_log: self._start_log("download", local_vars=locals()) - self.show_top_level_folder() + self._show_pre_transfer_messages() TransferData( self.cfg, @@ -476,7 +476,7 @@ def upload_specific_folder_or_file( """ self._start_log("upload_specific_folder_or_file", local_vars=locals()) - self.show_top_level_folder() + self._show_pre_transfer_messages() processed_filepath = utils.get_path_after_base_folder( self.cfg.get_base_folder("local"), @@ -529,7 +529,7 @@ def download_specific_folder_or_file( "download_specific_folder_or_file", local_vars=locals() ) - self.show_top_level_folder() + self._show_pre_transfer_messages() processed_filepath = utils.get_path_after_base_folder( self.cfg.get_base_folder("central"), @@ -548,8 +548,10 @@ def download_specific_folder_or_file( ds_logger.close_log_filehandler() - def _show_pre_transfer_messages(self): + def _show_pre_transfer_messages(self, include_top_level_folder=True): formatting.warn_on_inconsistent_sub_or_ses_leading_zeros(self.cfg) + if include_top_level_folder: + self.show_top_level_folder() # ------------------------------------------------------------------------- # SSH @@ -1051,6 +1053,8 @@ def _transfer_entire_project( direction : direction to transfer the data, either "upload" (from local to central) or "download" (from central to local). """ + self._show_pre_transfer_messages(include_top_level_folder=False) + transfer_all_func = ( self.upload_all if direction == "upload" else self.download_all ) diff --git a/datashuttle/utils/formatting.py b/datashuttle/utils/formatting.py index 010574bf5..5e8cabbe3 100644 --- a/datashuttle/utils/formatting.py +++ b/datashuttle/utils/formatting.py @@ -440,18 +440,18 @@ def inconsistent_num_leading_zeros( if all_num_lens != [] and not identical_elements(all_num_lens): most_common_len = statistics.mode(all_num_lens) - larger_than_most_common_has_leading_zeros = [ + larger_than_most_common_with_leading_zeros = [ num for num in all_numbers - if (int(num) > most_common_len and num_leading_zeros(num) != 0) + if (len(num) > most_common_len and num_leading_zeros(num) != 0) ] + less_than_most_common = [ - num for num in all_numbers if int(num) < most_common_len + num for num in all_numbers if len(num) < most_common_len ] - if ( - any(less_than_most_common) - or larger_than_most_common_has_leading_zeros + if any(less_than_most_common) or any( + larger_than_most_common_with_leading_zeros ): return True diff --git a/tests/tests_integration/test_formatting.py b/tests/tests_integration/test_formatting.py index 3a340aed3..23444f85d 100644 --- a/tests/tests_integration/test_formatting.py +++ b/tests/tests_integration/test_formatting.py @@ -1,4 +1,6 @@ import os.path +import shutil +import warnings import pytest import test_utils @@ -115,3 +117,77 @@ def test_warning_non_consecutive_numbers(self, project): == "A subject number has been skipped, currently " "used subject numbers are: [5, 10]" ) + + @pytest.mark.parametrize( + "bad_sub_name", + ["sub-03_date-123", "sub-0003_id-123", "sub-0999", "sub-0034"], + ) + def test_warn_on_inconsistent_leading_zeros_subjects( + self, project, bad_sub_name + ): + + project.make_sub_folders( + ["sub-001", "sub-010", "sub-100_date-20221314", "sub-1000"], + ["ses-001_id-1231"], + ) + + self.run_warn_on_consistentent_leading_zeros_sub_or_ses( + project, + bad_sub_name, + "ses-002", # an innocuous ses-name, placeholder. + ) + + @pytest.mark.parametrize( + "bad_ses_name", + ["ses-03_date-123", "ses-0003_id-123", "ses-0999", "ses-0034"], + ) + def test_warn_on_inconsistent_leading_zeros_sessions( + self, project, bad_ses_name + ): + # TODO: check with breakpints this is doing exactly what is expected!!! + # TODO: should probably check these are raised at the actual + # transfer / start up function not in this lower-level function. + project.make_sub_folders( + ["sub-001", "sub-002"], + [ + "ses-001_id-1231", + "ses-020_date-123123" "ses-200", + "ses-2000_id-12312", + ], + ) + + self.run_warn_on_consistentent_leading_zeros_sub_or_ses( + project, + "sub-002", + bad_ses_name, # an innocuous sub-name, placeholder. + ) + + def run_warn_on_consistentent_leading_zeros_sub_or_ses( + self, project, sub_name, ses_name + ): + + with warnings.catch_warnings(): + warnings.simplefilter("error") + project._show_pre_transfer_messages() + + project.make_sub_folders(sub_name, ses_name) + + self.check_inconsistent_sub_or_ses_level_warning(project, "sub") + + project.upload_all() + shutil.rmtree(project.cfg.get_base_folder("local") / sub_name) + + self.check_inconsistent_sub_or_ses_level_warning(project, "sub") + + def check_inconsistent_sub_or_ses_level_warning(self, project, sub_or_ses): + + with pytest.warns(UserWarning) as w: + project._show_pre_transfer_messages() + + assert ( + str(w[0].message) == f"Inconsistent number of leading zeros for " + f"{sub_or_ses} names in the project found. It is " + f"crucial these are made consistent as " + f"soon as possible to avoid unexpected " + f"behaviour of DataShuttle during data transfer." + ) diff --git a/tests/tests_unit/test_unit.py b/tests/tests_unit/test_unit.py index dd9fe5a30..b4d97a6f4 100644 --- a/tests/tests_unit/test_unit.py +++ b/tests/tests_unit/test_unit.py @@ -189,6 +189,16 @@ def test_get_value_from_bids_name_regexp(self): id = utils.get_value_from_key_regexp(bids_name, "id")[0] assert id == "3asd@523" + def test_num_leading_zeros(self): + """ + Check num_leading_zeros handles prefixed and non-prefixed + case from -1 to -(100x 0)1. + """ + for i in range(100): + assert formatting.num_leading_zeros("1".zfill(i + 1)) == i + assert formatting.num_leading_zeros("sub-" + "1".zfill(i + 1)) == i + assert formatting.num_leading_zeros("ses-" + "1".zfill(i + 1)) == i + # ---------------------------------------------------------------------- # Utlis # ----------------------------------------------------------------------