Make clear if error came from people or already selected

And name the file/gsheet-tab in more places

Author: Hamish Downer

src/sortition_algorithms/adapters.py

@@ -108,6 +108,14 @@ def _address_from_row(person: list[str]) -> tuple[str, ...]:
 
 
 class AbstractDataSource(abc.ABC):
+    @property
+    @abc.abstractmethod
+    def people_data_container(self) -> str: ...
+
+    @property
+    @abc.abstractmethod
+    def already_selected_data_container(self) -> str: ...
+
     @abc.abstractmethod
     @contextmanager
     def read_feature_data(
@@ -145,6 +153,11 @@ def customise_people_parse_error(
         self, error: ParseTableMultiError, headers: Sequence[str]
     ) -> SelectionMultilineError: ...
 
+    @abc.abstractmethod
+    def customise_already_selected_parse_error(
+        self, error: ParseTableMultiError, headers: Sequence[str]
+    ) -> SelectionMultilineError: ...
+
 
 class SelectionData:
     def __init__(self, data_source: AbstractDataSource, gen_rem_tab: bool = True) -> None:
@@ -182,6 +195,7 @@ def load_people(self, settings: Settings, features: FeatureCollection) -> tuple[
                     features=features,
                     settings=settings,
                     feature_column_name=self.feature_column_name,
+                    data_container=self.data_source.people_data_container,
                 )
             except ParseTableMultiError as error:
                 new_error = self.data_source.customise_people_parse_error(error, headers)
@@ -204,9 +218,10 @@ def load_already_selected(self, settings: Settings, features: FeatureCollection)
                     features=features,
                     settings=settings,
                     feature_column_name=self.feature_column_name,
+                    data_container=self.data_source.already_selected_data_container,
                 )
             except ParseTableMultiError as error:
-                new_error = self.data_source.customise_people_parse_error(error, headers)
+                new_error = self.data_source.customise_already_selected_parse_error(error, headers)
                 raise new_error from error
         return people, report
 
@@ -261,6 +276,14 @@ def __init__(self, features_data: str, people_data: str, already_selected_data:
         self.selected_file_written = False
         self.remaining_file_written = False
 
+    @property
+    def people_data_container(self) -> str:
+        return "people CSV data"
+
+    @property
+    def already_selected_data_container(self) -> str:
+        return "already selected CSV data"
+
     @contextmanager
     def read_feature_data(
         self, report: RunReport
@@ -315,6 +338,12 @@ def customise_people_parse_error(
         # given the info is in strings, we can't usefully add anything
         return error
 
+    def customise_already_selected_parse_error(
+        self, error: ParseTableMultiError, headers: Sequence[str]
+    ) -> SelectionMultilineError:
+        # given the info is in strings, we can't usefully add anything
+        return error
+
 
 class CSVFileDataSource(AbstractDataSource):
     def __init__(
@@ -332,6 +361,14 @@ def __init__(
         self.selected_file = selected_file
         self.remaining_file = remaining_file
 
+    @property
+    def people_data_container(self) -> str:
+        return f" CSV file '{self.people_file}'"
+
+    @property
+    def already_selected_data_container(self) -> str:
+        return f" CSV file '{self.already_selected_file}'"
+
     @contextmanager
     def read_feature_data(
         self, report: RunReport
@@ -395,6 +432,14 @@ def customise_people_parse_error(
             *[str(e) for e in error.all_errors],
         ])
 
+    def customise_already_selected_parse_error(
+        self, error: ParseTableMultiError, headers: Sequence[str]
+    ) -> SelectionMultilineError:
+        return SelectionMultilineError([
+            f"Parser error(s) while reading already selected people from {self.already_selected_file}",
+            *[str(e) for e in error.all_errors],
+        ])
+
 
 class GSheetTabNamer:
     def __init__(self) -> None:
@@ -463,6 +508,14 @@ def __init__(
         self.tab_namer = GSheetTabNamer()
         self._report = RunReport()
 
+    @property
+    def people_data_container(self) -> str:
+        return f"'{self.people_tab_name}' tab"
+
+    @property
+    def already_selected_data_container(self) -> str:
+        return f"'{self.already_selected_tab_name}' tab"
+
     @property
     def client(self) -> gspread.client.Client:
         if self._client is None:
@@ -719,14 +772,22 @@ def customise_features_parse_error(
         self, error: ParseTableMultiError, headers: Sequence[str]
     ) -> SelectionMultilineError:
         return SelectionMultilineError([
-            f"Parser error(s) while reading features from {self.feature_tab_name} worksheet",
+            f"Parser error(s) while reading features from '{self.feature_tab_name}' worksheet",
             *self._annotate_parse_errors_with_cell_names(error, headers),
         ])
 
     def customise_people_parse_error(
         self, error: ParseTableMultiError, headers: Sequence[str]
     ) -> SelectionMultilineError:
         return SelectionMultilineError([
-            f"Parser error(s) while reading people from {self.people_tab_name} worksheet",
+            f"Parser error(s) while reading people from '{self.people_tab_name}' worksheet",
+            *self._annotate_parse_errors_with_cell_names(error, headers),
+        ])
+
+    def customise_already_selected_parse_error(
+        self, error: ParseTableMultiError, headers: Sequence[str]
+    ) -> SelectionMultilineError:
+        return SelectionMultilineError([
+            f"Parser error(s) while reading people from '{self.already_selected_tab_name}' worksheet",
             *self._annotate_parse_errors_with_cell_names(error, headers),
         ])

src/sortition_algorithms/people.py

@@ -148,30 +148,35 @@ def find_person_by_position_in_category(self, feature_name: str, feature_value:
 
 
 # simple helper function to tidy the code below
-def _check_columns_exist_or_multiple(people_head: list[str], column_list: Iterable[str], error_label: str) -> None:
+def _check_columns_exist_or_multiple(
+    people_head: list[str], column_list: Iterable[str], error_label: str, data_container: str = "people data"
+) -> None:
     people_head_lower = [h.lower() for h in people_head]
     for column in column_list:
         column = column.lower()
         column_count = people_head_lower.count(column)
         if column_count == 0:
-            msg = f"No '{column}' column {error_label} found in people data!"
+            msg = f"No '{column}' column {error_label} found in {data_container}!"
             raise BadDataError(msg)
         elif column_count > 1:
-            msg = f"MORE THAN 1 '{column}' column {error_label} found in people data!"
+            msg = f"MORE THAN 1 '{column}' column {error_label} found in {data_container}!"
             raise BadDataError(msg)
 
 
-def _check_people_head(people_head: list[str], features: FeatureCollection, settings: Settings) -> None:
+def _check_people_head(
+    people_head: list[str], features: FeatureCollection, settings: Settings, data_container: str = "people data"
+) -> None:
     # check that id_column and all the features, columns_to_keep and
     # check_same_address_columns are in the people data fields...
     # check both for existence and duplicate column names
-    _check_columns_exist_or_multiple(people_head, [settings.id_column], "(unique id)")
-    _check_columns_exist_or_multiple(people_head, list(features.keys()), "(a feature)")
-    _check_columns_exist_or_multiple(people_head, settings.columns_to_keep, "(to keep)")
+    _check_columns_exist_or_multiple(people_head, [settings.id_column], "(unique id)", data_container)
+    _check_columns_exist_or_multiple(people_head, list(features.keys()), "(a feature)", data_container)
+    _check_columns_exist_or_multiple(people_head, settings.columns_to_keep, "(to keep)", data_container)
     _check_columns_exist_or_multiple(
         people_head,
         settings.check_same_address_columns,
         "(to check same address)",
+        data_container,
     )
 
 
@@ -256,9 +261,10 @@ def read_in_people(
     features: FeatureCollection,
     settings: Settings,
     feature_column_name: str = "feature",
+    data_container: str = "people data",
 ) -> tuple[People, RunReport]:
     report = RunReport()
-    _check_people_head(people_head, features, settings)
+    _check_people_head(people_head, features, settings, data_container)
     # we need to iterate through more than once, so save as list here
     stripped_people_body = [normalise_dict(row) for row in people_body]
     report.add_lines(check_for_duplicate_people(stripped_people_body, settings))

tests/test_adapters.py

@@ -681,7 +681,7 @@ def test_gsheet_customise_features_parse_error():
     )
     assert "Some error - see cell B4" in str(new_error)
     assert "Min greater than max - see cells C6 D6" in str(new_error)
-    assert "Categories worksheet" in str(new_error)
+    assert "'Categories' worksheet" in str(new_error)
 
 
 def test_gsheet_customise_people_parse_error():
@@ -695,7 +695,7 @@ def test_gsheet_customise_people_parse_error():
         parse_features_error, headers=("nationbuilder_id", "name", "gender")
     )
     assert "Another error - see cell C4" in str(new_error)
-    assert "Respondents worksheet" in str(new_error)
+    assert "'Respondents' worksheet" in str(new_error)
 
 
 def test_csv_load_feature_from_file_failure(tmp_path: Path):

tests/test_people.py

@@ -311,9 +311,19 @@ def test_read_in_people_missing_feature_column(self):
         # Remove gender column from header
         people_head.remove("gender")
 
-        with pytest.raises(errors.BadDataError, match="No 'gender' column \\(a feature\\) found"):
+        with pytest.raises(errors.BadDataError, match="No 'gender' column \\(a feature\\) found in people data"):
             read_in_people(people_head, people_body, features, settings)
 
+    def test_read_in_people_missing_feature_column_custom_container(self):
+        """Test read_in_people with missing feature column."""
+        features, settings, people_head, people_body = self.create_test_data()
+
+        # Remove gender column from header
+        people_head.remove("gender")
+
+        with pytest.raises(errors.BadDataError, match="No 'gender' column \\(a feature\\) found in dusty corner"):
+            read_in_people(people_head, people_body, features, settings, data_container="dusty corner")
+
     def test_read_in_people_missing_columns_to_keep(self):
         """Test read_in_people with missing columns_to_keep."""
         features, settings, people_head, people_body = self.create_test_data()