Add tests to check warning raised for inconsistent sub or ses leading…

… zeros.

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
         )

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
 

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."
+        )

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
     # ----------------------------------------------------------------------