diff --git a/dcicutils/submitr/custom_excel.py b/dcicutils/submitr/custom_excel.py index a9eef987d..37e90777b 100644 --- a/dcicutils/submitr/custom_excel.py +++ b/dcicutils/submitr/custom_excel.py @@ -5,6 +5,7 @@ from typing import Any, List, Optional from dcicutils.data_readers import Excel, ExcelSheetReader from dcicutils.misc_utils import to_boolean, to_float, to_integer +from dcicutils.submitr.donor_transformer import ProtectedDonorWorkbookTransformer # This module implements a custom Excel spreadsheet class which supports "custom column mappings", # meaning that, at a very low/early level in processing, the columns/values in the spreadsheet @@ -100,22 +101,36 @@ def _array_name_of(synthetic_column_name: str) -> Optional[str]: class CustomExcel(Excel): - def __init__(self, *args, portal=None, **kwargs): + _saved_transformed_workbook_path_pairs = set() + + def __init__(self, *args, portal=None, transform_protected_donor: bool = False, + transformed_workbook_path: Optional[str] = None, **kwargs): + self._transform_protected_donor = bool(transform_protected_donor) + self._transformed_workbook_path = transformed_workbook_path super().__init__(*args, **kwargs) + if self._transform_protected_donor: + transformed = ProtectedDonorWorkbookTransformer( + effective_sheet_name=self.effective_sheet_name).transform(self._workbook) + self.sheet_names = [sheet_name for sheet_name in self._workbook.sheetnames + if not self.is_hidden_sheet(self._workbook[sheet_name])] + if transformed and self._transformed_workbook_path: + self._save_transformed_workbook(self._transformed_workbook_path) self._custom_column_mappings = CustomExcel._get_custom_column_mappings(portal=portal) @classmethod - def with_portal(cls, portal): - """Return a subclass of CustomExcel with portal baked in. + def with_portal(cls, portal, **options): + """Return a subclass of CustomExcel with portal/options baked in. Use this when passing excel_class to StructuredDataSet, which requires a real class (it calls issubclass() on the argument internally): - excel_class=CustomExcel.with_portal(portal) + excel_class=CustomExcel.with_portal(portal, transform_protected_donor=False) """ class _CustomExcelWithPortal(cls): def __init__(self, *args, **kwargs): kwargs.setdefault("portal", portal) + for key, value in options.items(): + kwargs.setdefault(key, value) super().__init__(*args, **kwargs) _CustomExcelWithPortal.__name__ = "CustomExcel" _CustomExcelWithPortal.__qualname__ = "CustomExcel" @@ -131,6 +146,25 @@ def effective_sheet_name(sheet_name: str) -> str: return sheet_name[underscore + 1:] return sheet_name + def _save_transformed_workbook(self, path: str) -> None: + path = os.path.abspath(os.path.expanduser(path)) + input_path = os.path.abspath(os.path.expanduser(self._file)) if self._file else None + if not path.lower().endswith(".xlsx"): + raise ValueError(f"Transformed workbook output path must end with .xlsx: {path}") + if input_path and path == input_path: + raise ValueError(f"Transformed workbook output path must differ from input workbook path: {path}") + directory = os.path.dirname(path) + if not os.path.isdir(directory): + raise ValueError(f"Directory for transformed workbook output does not exist: {directory}") + path_pair = (input_path, path) + if os.path.exists(path) and path_pair not in CustomExcel._saved_transformed_workbook_path_pairs: + raise ValueError(f"Transformed workbook output path already exists: {path}") + try: + self._workbook.save(path) + CustomExcel._saved_transformed_workbook_path_pairs.add(path_pair) + except Exception as e: + raise ValueError(f"Cannot save transformed workbook to {path}: {e}") from e + @staticmethod def _get_custom_column_mappings(portal=None) -> Optional[dict]: diff --git a/dcicutils/submitr/donor_transformer.py b/dcicutils/submitr/donor_transformer.py new file mode 100644 index 000000000..67514f92e --- /dev/null +++ b/dcicutils/submitr/donor_transformer.py @@ -0,0 +1,226 @@ +from typing import Callable, List, Optional + +import openpyxl +from openpyxl.worksheet.worksheet import Worksheet + + +DONOR_SHEET = "Donor" +PROTECTED_DONOR_SHEET = "ProtectedDonor" +SUBMITTED_ID_COLUMN = "submitted_id" +PROTECTED_DONOR_COLUMN = "protected_donor" +DONOR_LINK_COLUMN = "donor" +DONOR_TOKEN = "_DONOR_" +PROTECTED_DONOR_TOKEN = "_PROTECTED-DONOR_" +PROTECTED_DONOR_STATUS = "in review" +PROTECTED_REFERENCE_SHEETS = { + "Demographic", + "DeathCircumstances", + "FamilyHistory", + "MedicalHistory", + "TissueCollection", +} +SERVER_MANAGED_COLUMNS = { + "accession", + "uuid", + "alternate_accessions", +} + + +class ProtectedDonorTransformError(ValueError): + pass + + +def to_protected_donor_submitted_id(submitted_id: Optional[str]) -> Optional[str]: + if not isinstance(submitted_id, str) or DONOR_TOKEN not in submitted_id: + raise ProtectedDonorTransformError( + f"Cannot derive ProtectedDonor submitted_id from {submitted_id!r}; expected token {DONOR_TOKEN!r}." + ) + return submitted_id.replace(DONOR_TOKEN, PROTECTED_DONOR_TOKEN, 1) + + +class ProtectedDonorWorkbookTransformer: + """Transforms SMaHT donor workbooks in memory for ProtectedDonor ingestion.""" + + def __init__(self, effective_sheet_name: Optional[Callable[[str], str]] = None) -> None: + self._effective_sheet_name = effective_sheet_name or (lambda sheet_name: sheet_name) + + def transform(self, workbook: openpyxl.Workbook) -> bool: + transformed = False + donor_sheets = self._visible_sheets_by_effective_name(workbook, DONOR_SHEET) + if not donor_sheets: + return transformed + for donor_sheet in donor_sheets: + transformed = self._transform_donor_sheet(workbook, donor_sheet) or transformed + for sheet in self._visible_sheets(workbook): + if self._effective_sheet_name(sheet.title) in PROTECTED_REFERENCE_SHEETS: + transformed = self._rewrite_donor_links(sheet) or transformed + return transformed + + def _transform_donor_sheet(self, workbook: openpyxl.Workbook, donor_sheet: Worksheet) -> bool: + headers = self._headers(donor_sheet) + submitted_id_column = self._column_index(headers, SUBMITTED_ID_COLUMN) + if submitted_id_column is None: + return False + protected_donor_column = self._ensure_column(donor_sheet, headers, PROTECTED_DONOR_COLUMN) + expected_rows = [] + for row_number in range(2, donor_sheet.max_row + 1): + donor_id = donor_sheet.cell(row_number, submitted_id_column).value + if donor_id in [None, ""]: + continue + protected_id = to_protected_donor_submitted_id(str(donor_id).strip()) + existing = donor_sheet.cell(row_number, protected_donor_column).value + if existing not in [None, ""] and str(existing).strip() != protected_id: + raise ProtectedDonorTransformError( + f"Donor row {row_number} protected_donor value {existing!r} does not match " + f"expected {protected_id!r}." + ) + donor_sheet.cell(row_number, protected_donor_column).value = protected_id + expected_rows.append(self._protected_row_from_donor_row(donor_sheet, row_number, headers, protected_id)) + if not expected_rows: + return False + protected_sheet_name = self._protected_sheet_name_for(donor_sheet.title) + if protected_sheet_name in workbook.sheetnames: + self._validate_existing_protected_donor_sheet(workbook[protected_sheet_name], expected_rows) + else: + self._create_protected_donor_sheet(workbook, protected_sheet_name, expected_rows) + return True + + def _protected_sheet_name_for(self, donor_sheet_name: str) -> str: + if self._effective_sheet_name(donor_sheet_name) == DONOR_SHEET: + prefix, separator, _ = donor_sheet_name.rpartition("_") + if separator and prefix: + return f"{prefix}_{PROTECTED_DONOR_SHEET}" + return PROTECTED_DONOR_SHEET + + def _protected_row_from_donor_row(self, sheet: Worksheet, row_number: int, + headers: List[str], protected_id: str) -> dict: + row = {} + for column_number, header in enumerate(headers, start=1): + if not header or header in SERVER_MANAGED_COLUMNS or header == PROTECTED_DONOR_COLUMN: + continue + value = sheet.cell(row_number, column_number).value + if header == SUBMITTED_ID_COLUMN: + value = protected_id + elif header == "status": + # ProtectedDonor submissions should enter the portal as in review, regardless + # of any public Donor status value in the source workbook. + value = PROTECTED_DONOR_STATUS + row[header] = value + if "status" not in row: + row["status"] = PROTECTED_DONOR_STATUS + return row + + def _create_protected_donor_sheet(self, workbook: openpyxl.Workbook, + sheet_name: str, rows: List[dict]) -> None: + sheet = workbook.create_sheet(sheet_name) + headers = list(rows[0].keys()) + for column_number, header in enumerate(headers, start=1): + sheet.cell(1, column_number).value = header + for row_number, row in enumerate(rows, start=2): + for column_number, header in enumerate(headers, start=1): + sheet.cell(row_number, column_number).value = row.get(header) + + def _validate_existing_protected_donor_sheet(self, sheet: Worksheet, expected_rows: List[dict]) -> None: + headers = self._headers(sheet) + submitted_id_column = self._column_index(headers, SUBMITTED_ID_COLUMN) + if submitted_id_column is None: + raise ProtectedDonorTransformError( + f"Existing {sheet.title!r} sheet is missing required {SUBMITTED_ID_COLUMN!r} column." + ) + actual_by_id = {} + for row_number in range(2, sheet.max_row + 1): + submitted_id = sheet.cell(row_number, submitted_id_column).value + if submitted_id in [None, ""]: + continue + actual_by_id[str(submitted_id).strip()] = self._row_dict(sheet, row_number, headers) + expected_ids = {expected.get(SUBMITTED_ID_COLUMN) for expected in expected_rows} + unexpected_ids = set(actual_by_id.keys()) - expected_ids + if unexpected_ids: + raise ProtectedDonorTransformError( + f"Existing {sheet.title!r} sheet contains unexpected ProtectedDonor submitted_id(s): " + f"{', '.join(sorted(unexpected_ids))}." + ) + for expected in expected_rows: + expected_id = expected.get(SUBMITTED_ID_COLUMN) + actual = actual_by_id.get(expected_id) + if actual is None: + raise ProtectedDonorTransformError( + f"Existing {sheet.title!r} sheet is missing expected ProtectedDonor {expected_id!r}." + ) + for key, expected_value in expected.items(): + if key not in actual: + continue + actual_value = actual.get(key) + if self._normalized(actual_value) != self._normalized(expected_value): + raise ProtectedDonorTransformError( + f"Existing {sheet.title!r} row for {expected_id!r} has {key!r} value " + f"{actual_value!r}; expected {expected_value!r}." + ) + + def _rewrite_donor_links(self, sheet: Worksheet) -> bool: + transformed = False + headers = self._headers(sheet) + donor_column = self._column_index(headers, DONOR_LINK_COLUMN) + if donor_column is None: + return transformed + for row_number in range(2, sheet.max_row + 1): + value = sheet.cell(row_number, donor_column).value + if isinstance(value, str) and DONOR_TOKEN in value: + sheet.cell(row_number, donor_column).value = to_protected_donor_submitted_id(value) + transformed = True + return transformed + + def _visible_sheets_by_effective_name(self, workbook: openpyxl.Workbook, effective_name: str) -> List[Worksheet]: + return [sheet for sheet in self._visible_sheets(workbook) + if self._effective_sheet_name(sheet.title) == effective_name] + + @staticmethod + def _visible_sheets(workbook: openpyxl.Workbook) -> List[Worksheet]: + return [sheet for sheet in workbook.worksheets if not ProtectedDonorWorkbookTransformer._is_hidden_sheet(sheet)] + + @staticmethod + def _is_hidden_sheet(sheet: Worksheet) -> bool: + title = sheet.title + if sheet.sheet_state == "hidden": + return True + return ((title.startswith("(") and title.endswith(")")) or + (title.startswith("[") and title.endswith("]")) or + (title.startswith("{") and title.endswith("}")) or + (title.startswith("<") and title.endswith(">"))) + + @staticmethod + def _headers(sheet: Worksheet) -> List[str]: + headers = [] + for cell in sheet[1]: + value = cell.value + if value is None or not str(value).strip(): + break + headers.append(str(value).strip()) + return headers + + @staticmethod + def _column_index(headers: List[str], column_name: str) -> Optional[int]: + try: + return headers.index(column_name) + 1 + except ValueError: + return None + + @staticmethod + def _ensure_column(sheet: Worksheet, headers: List[str], column_name: str) -> int: + if (column_index := ProtectedDonorWorkbookTransformer._column_index(headers, column_name)) is not None: + return column_index + column_index = len(headers) + 1 + sheet.cell(1, column_index).value = column_name + headers.append(column_name) + return column_index + + @staticmethod + def _row_dict(sheet: Worksheet, row_number: int, headers: List[str]) -> dict: + return {header: sheet.cell(row_number, column_number).value + for column_number, header in enumerate(headers, start=1)} + + @staticmethod + def _normalized(value): + if value is None: + return "" + return str(value).strip() diff --git a/pyproject.toml b/pyproject.toml index 031c2913a..385687c00 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "dcicutils" -version = "8.19.0" +version = "8.19.0b1" description = "Utility package for interacting with the 4DN Data Portal and other 4DN resources" authors = ["4DN-DCIC Team "] license = "MIT" diff --git a/test/test_protected_donor_transform.py b/test/test_protected_donor_transform.py new file mode 100644 index 000000000..94259a422 --- /dev/null +++ b/test/test_protected_donor_transform.py @@ -0,0 +1,216 @@ +import openpyxl +import pytest + +from dcicutils.structured_data import StructuredDataSet +from dcicutils.submitr.custom_excel import CustomExcel +from dcicutils.submitr.donor_transformer import ( + ProtectedDonorTransformError, + to_protected_donor_submitted_id, +) + + +def _write_workbook(path, sheets): + workbook = openpyxl.Workbook() + default = workbook.active + workbook.remove(default) + for sheet_name, rows in sheets.items(): + sheet = workbook.create_sheet(sheet_name) + for row in rows: + sheet.append(row) + workbook.save(path) + + +def _load(path, excel_class=None): + if excel_class is None: + excel_class = CustomExcel.with_portal(None, transform_protected_donor=True) + return StructuredDataSet(file=str(path), portal=None, excel_class=excel_class).data + + +def test_to_protected_donor_submitted_id_replaces_only_donor_token(): + assert to_protected_donor_submitted_id("ABC_DONOR_1234") == "ABC_PROTECTED-DONOR_1234" + assert to_protected_donor_submitted_id("ABC_DONOR_1234_DONOR_X") == "ABC_PROTECTED-DONOR_1234_DONOR_X" + with pytest.raises(ProtectedDonorTransformError, match="expected token"): + to_protected_donor_submitted_id("ABC-DONOR-1234") + + +def test_protected_donor_transform_generates_sheet_and_rewrites_protected_refs(tmp_path): + path = tmp_path / "donor.xlsx" + _write_workbook(path, { + "Donor": [ + ["submitted_id", "external_id", "sex", "age", "status"], + ["ABC_DONOR_0001", "EXT-1", "Female", "45", "released"], + ], + "Demographic": [ + ["submitted_id", "donor", "race"], + ["ABC_DEMOGRAPHIC_0001", "ABC_DONOR_0001", "reported unknown"], + ], + "MedicalHistory": [ + ["submitted_id", "donor", "tobacco_use"], + ["ABC_MEDICAL-HISTORY_0001", "ABC_DONOR_0001", "No"], + ], + "Tissue": [ + ["submitted_id", "donor", "uberon_id"], + ["ABC_TISSUE_0001", "ABC_DONOR_0001", "UBERON:0000955"], + ], + }) + + data = _load(path) + + assert data["Donor"] == [{ + "submitted_id": "ABC_DONOR_0001", + "external_id": "EXT-1", + "sex": "Female", + "age": "45", + "status": "released", + "protected_donor": "ABC_PROTECTED-DONOR_0001", + }] + assert data["ProtectedDonor"] == [{ + "submitted_id": "ABC_PROTECTED-DONOR_0001", + "external_id": "EXT-1", + "sex": "Female", + "age": "45", + "status": "in review", + }] + assert data["Demographic"][0]["donor"] == "ABC_PROTECTED-DONOR_0001" + assert data["MedicalHistory"][0]["donor"] == "ABC_PROTECTED-DONOR_0001" + assert data["Tissue"][0]["donor"] == "ABC_DONOR_0001" + + +@pytest.mark.parametrize("sheet_name", [ + "Demographic", + "DeathCircumstances", + "FamilyHistory", + "MedicalHistory", + "TissueCollection", +]) +def test_protected_donor_transform_rewrites_all_protected_reference_sheets(tmp_path, sheet_name): + path = tmp_path / f"{sheet_name}.xlsx" + _write_workbook(path, { + "Donor": [["submitted_id"], ["ABC_DONOR_0001"]], + sheet_name: [["submitted_id", "donor"], [f"ABC_{sheet_name.upper()}_0001", "ABC_DONOR_0001"]], + }) + + data = _load(path) + + assert data[sheet_name][0]["donor"] == "ABC_PROTECTED-DONOR_0001" + + +def test_protected_donor_transform_ignores_parenthesized_sheets(tmp_path): + path = tmp_path / "hidden_donor.xlsx" + _write_workbook(path, { + "(Donor)": [["submitted_id"], ["ABC_DONOR_0001"]], + "Tissue": [["submitted_id", "donor"], ["ABC_TISSUE_0001", "ABC_DONOR_0001"]], + }) + + data = _load(path) + + assert "Donor" not in data + assert "ProtectedDonor" not in data + assert data["Tissue"][0]["donor"] == "ABC_DONOR_0001" + + +def test_protected_donor_transform_validates_existing_protected_donor_sheet(tmp_path): + path = tmp_path / "existing_valid.xlsx" + _write_workbook(path, { + "Donor": [["submitted_id", "external_id", "sex"], ["ABC_DONOR_0001", "EXT-1", "Female"]], + "ProtectedDonor": [["submitted_id", "external_id", "sex", "status"], + ["ABC_PROTECTED-DONOR_0001", "EXT-1", "Female", "in review"]], + }) + + data = _load(path) + + assert len(data["ProtectedDonor"]) == 1 + assert data["Donor"][0]["protected_donor"] == "ABC_PROTECTED-DONOR_0001" + + +def test_protected_donor_transform_rejects_mismatched_existing_protected_donor_sheet(tmp_path): + path = tmp_path / "existing_invalid.xlsx" + _write_workbook(path, { + "Donor": [["submitted_id", "external_id", "sex"], ["ABC_DONOR_0001", "EXT-1", "Female"]], + "ProtectedDonor": [["submitted_id", "external_id", "sex", "status"], + ["ABC_PROTECTED-DONOR_0001", "DIFFERENT", "Female", "in review"]], + }) + + with pytest.raises(ProtectedDonorTransformError, match="external_id"): + _load(path) + + +def test_protected_donor_transform_rejects_donor_id_without_donor_token(tmp_path): + path = tmp_path / "bad_donor_id.xlsx" + _write_workbook(path, { + "Donor": [["submitted_id"], ["ABC-DONOR-0001"]], + }) + + with pytest.raises(ProtectedDonorTransformError, match="expected token"): + _load(path) + + +def test_protected_donor_transform_can_save_transformed_workbook(tmp_path): + path = tmp_path / "input.xlsx" + output = tmp_path / "transformed.xlsx" + _write_workbook(path, { + "Donor": [["submitted_id"], ["ABC_DONOR_0001"]], + "Demographic": [["submitted_id", "donor"], ["ABC_DEMOGRAPHIC_0001", "ABC_DONOR_0001"]], + }) + + data = _load(path, excel_class=CustomExcel.with_portal(None, transform_protected_donor=True, transformed_workbook_path=str(output))) + saved_data = _load(output, excel_class=CustomExcel.with_portal(None, transform_protected_donor=False)) + + assert output.exists() + assert data == saved_data + assert saved_data["ProtectedDonor"][0]["submitted_id"] == "ABC_PROTECTED-DONOR_0001" + assert saved_data["Demographic"][0]["donor"] == "ABC_PROTECTED-DONOR_0001" + + +def test_protected_donor_transform_rejects_output_path_same_as_input(tmp_path): + path = tmp_path / "same.xlsx" + _write_workbook(path, {"Donor": [["submitted_id"], ["ABC_DONOR_0001"]]}) + + with pytest.raises(ValueError, match="must differ from input"): + _load(path, excel_class=CustomExcel.with_portal(None, transform_protected_donor=True, transformed_workbook_path=str(path))) + + +def test_protected_donor_transform_rejects_existing_output_path(tmp_path): + path = tmp_path / "input.xlsx" + output = tmp_path / "existing.xlsx" + _write_workbook(path, {"Donor": [["submitted_id"], ["ABC_DONOR_0001"]]}) + output.write_text("do not overwrite") + + with pytest.raises(ValueError, match="already exists"): + _load(path, excel_class=CustomExcel.with_portal(None, transform_protected_donor=True, transformed_workbook_path=str(output))) + + +def test_protected_donor_transform_rejects_reusing_output_for_different_input(tmp_path): + path1 = tmp_path / "input1.xlsx" + path2 = tmp_path / "input2.xlsx" + output = tmp_path / "transformed.xlsx" + _write_workbook(path1, {"Donor": [["submitted_id"], ["ABC_DONOR_0001"]]}) + _write_workbook(path2, {"Donor": [["submitted_id"], ["ABC_DONOR_0002"]]}) + + _load(path1, excel_class=CustomExcel.with_portal(None, transform_protected_donor=True, transformed_workbook_path=str(output))) + with pytest.raises(ValueError, match="already exists"): + _load(path2, excel_class=CustomExcel.with_portal(None, transform_protected_donor=True, transformed_workbook_path=str(output))) + + +def test_protected_donor_transform_does_not_save_when_no_transform_occurs(tmp_path): + path = tmp_path / "no_donor.xlsx" + output = tmp_path / "not_created.xlsx" + _write_workbook(path, {"Tissue": [["submitted_id", "donor"], ["ABC_TISSUE_0001", "ABC_DONOR_0001"]]}) + + _load(path, excel_class=CustomExcel.with_portal(None, transform_protected_donor=True, transformed_workbook_path=str(output))) + + assert not output.exists() + + +def test_protected_donor_transform_can_be_disabled(tmp_path): + path = tmp_path / "disabled.xlsx" + _write_workbook(path, { + "Donor": [["submitted_id"], ["ABC_DONOR_0001"]], + "Demographic": [["submitted_id", "donor"], ["ABC_DEMOGRAPHIC_0001", "ABC_DONOR_0001"]], + }) + + data = _load(path, excel_class=CustomExcel.with_portal(None, transform_protected_donor=False)) + + assert "ProtectedDonor" not in data + assert "protected_donor" not in data["Donor"][0] + assert data["Demographic"][0]["donor"] == "ABC_DONOR_0001"