From a05973254e31e1cc35661c3416f1fef64a848d4b Mon Sep 17 00:00:00 2001 From: Estelle Da Date: Fri, 9 Oct 2026 15:42:24 +1100 Subject: [PATCH 1/8] Solve the bug of column name is accession. --- src/mavedb/lib/csv/columns.py | 17 ++++++++++- src/mavedb/lib/csv/specs.py | 9 ++++-- src/mavedb/lib/csv/variant.py | 9 +++--- .../lib/validation/constants/general.py | 1 + .../lib/validation/dataframe/dataframe.py | 4 +++ tests/lib/csv/test_columns.py | 30 +++++++++++++++++++ tests/lib/csv/test_variant.py | 13 ++++++++ tests/validation/dataframe/test_dataframe.py | 27 +++++++++++++++++ 8 files changed, 103 insertions(+), 7 deletions(-) diff --git a/src/mavedb/lib/csv/columns.py b/src/mavedb/lib/csv/columns.py index 2634c14b3..732db6ce6 100644 --- a/src/mavedb/lib/csv/columns.py +++ b/src/mavedb/lib/csv/columns.py @@ -59,6 +59,15 @@ def _value_or_na(value: Any, na_rep: str = NA_VALUE) -> str: return str(value) +_CORE_COLUMN_NAMES = frozenset(namespace_spec(CORE_NAMESPACE).columns({})) # type: ignore[union-attr] +"""Core's own column names, so an investigator column of the same name can still be told apart. + +In practice only ``accession`` can collide: upload strips the HGVS columns out of the investigator's own +score and count data (``create_variants_data`` in ``lib/score_sets.py``) before it is ever stored, so +they never reach this module as a dataset column. +""" + + def _format_column_key(namespace: str, column_key: str, namespaced: bool = False) -> str: """Shared key-formatting logic used by both header assembly and row assembly.""" # Always namespaced regardless of the caller's preference: the release or calibration URN is what @@ -69,7 +78,9 @@ def _format_column_key(namespace: str, column_key: str, namespaced: bool = False if namespace == CORE_NAMESPACE: # core is never namespaced return column_key - if namespaced: + # Forced even for un-namespaced output: a dataset column sharing a core column's name would otherwise + # collide with it once the prefix is stripped. The core column itself keeps its bare name. + if namespaced or column_key in _CORE_COLUMN_NAMES: spec = namespace_spec(namespace) prefix = spec.emit_under if spec is not None and spec.emit_under is not None else namespace return f"{prefix}.{column_key}" @@ -109,6 +120,10 @@ def plan_csv_columns(dataset_columns: dict, namespaces: list[str]) -> CsvColumnP def assemble_csv_headers(namespaced_columns: dict[str, list[str]], namespaced: bool = False) -> list[str]: """Build the flat column-header list from the namespace dict. + A dataset column named the same as a core column (only ``accession`` can happen in practice) keeps its + namespace prefix even in un-namespaced output, so it reads as e.g. ``scores.accession`` rather than + colliding with the bare ``accession`` core column. + Raises: ValueError: if two namespaces resolve to the same header. Un-namespaced output strips the prefix that would otherwise keep them apart, so requesting two namespaces that share a column name diff --git a/src/mavedb/lib/csv/specs.py b/src/mavedb/lib/csv/specs.py index bd37aa240..6399e25e3 100644 --- a/src/mavedb/lib/csv/specs.py +++ b/src/mavedb/lib/csv/specs.py @@ -7,7 +7,12 @@ from mavedb.lib.csv.namespaces import CALIBRATION_NS_PATTERN, CLINVAR_NS_PATTERN, CsvNamespace from mavedb.lib.mave.constants import REQUIRED_SCORE_COLUMN -from mavedb.lib.validation.constants.general import hgvs_nt_column, hgvs_pro_column, hgvs_splice_column +from mavedb.lib.validation.constants.general import ( + accession_column, + hgvs_nt_column, + hgvs_pro_column, + hgvs_splice_column, +) from mavedb.models.variant import Variant @@ -142,7 +147,7 @@ def _optional(getter: Callable) -> Callable: CORE_NAMESPACE: CsvNamespaceSpec( source=RowSource.VARIANT, resolvers={ - "accession": attrgetter("urn"), + accession_column: attrgetter("urn"), hgvs_nt_column: attrgetter(hgvs_nt_column), hgvs_splice_column: attrgetter(hgvs_splice_column), hgvs_pro_column: attrgetter(hgvs_pro_column), diff --git a/src/mavedb/lib/csv/variant.py b/src/mavedb/lib/csv/variant.py index 932abea74..5cdd207cd 100644 --- a/src/mavedb/lib/csv/variant.py +++ b/src/mavedb/lib/csv/variant.py @@ -38,6 +38,7 @@ calibration_namespace_for_urn, clinvar_namespace_sort_key, ) +from mavedb.lib.exceptions import NonexistentIdentifierError from mavedb.lib.mave.utils import NA_VALUE from mavedb.lib.urns import score_set_urn_sort_key, variant_urn_sort_key from mavedb.lib.permissions.score_calibration import ScoreCalibrationViewer @@ -255,14 +256,14 @@ def available_variant_csv_namespaces( equivalent measurements, and one ``clinvar.YYYY_MM`` per release covering them. Raises: - ValueError: if no variant with *variant_urn* exists. + NonexistentIdentifierError: if no variant with *variant_urn* exists. """ measurements = _equivalent_measurements(db, variant_urn, may_read_score_set=may_read_score_set, as_of=as_of) if measurements is None: variant = db.scalars(select(Variant).where(Variant.urn == variant_urn).limit(1)).first() if variant is None: - raise ValueError(f"variant with URN '{variant_urn}' not found") + raise NonexistentIdentifierError(f"variant with URN '{variant_urn}' not found") # No mapping on this variant, but its score set may still be mapped, in which case the # mapping-derived columns are owed with NA rather than omitted. @@ -306,7 +307,7 @@ def get_variant_csv( instant. Defaults to currently-live rows. Raises: - ValueError: if no variant with *variant_urn* exists. + NonexistentIdentifierError: if no variant with *variant_urn* exists. """ measurements = _equivalent_measurements(db, variant_urn, may_read_score_set=may_read_score_set, as_of=as_of) @@ -382,7 +383,7 @@ def _unmapped_variant_csv( ).one_or_none() if variant is None: - raise ValueError(f"variant with URN '{variant_urn}' not found") + raise NonexistentIdentifierError(f"variant with URN '{variant_urn}' not found") plan = plan_csv_columns( dataset_columns={}, diff --git a/src/mavedb/lib/validation/constants/general.py b/src/mavedb/lib/validation/constants/general.py index 14a087fc7..a52e22860 100644 --- a/src/mavedb/lib/validation/constants/general.py +++ b/src/mavedb/lib/validation/constants/general.py @@ -36,6 +36,7 @@ hgvs_splice_column = "hgvs_splice" hgvs_pro_column = "hgvs_pro" guide_sequence_column = "guide_sequence" +accession_column = "accession" hgvs_columns = sorted([hgvs_nt_column, hgvs_pro_column, hgvs_splice_column]) meta_data = "meta_data" score_columns = "score_columns" diff --git a/src/mavedb/lib/validation/dataframe/dataframe.py b/src/mavedb/lib/validation/dataframe/dataframe.py index dcf6c8e59..9543971b2 100644 --- a/src/mavedb/lib/validation/dataframe/dataframe.py +++ b/src/mavedb/lib/validation/dataframe/dataframe.py @@ -5,6 +5,7 @@ from mavedb.lib.exceptions import MixedTargetError from mavedb.lib.validation.constants.general import ( + accession_column, guide_sequence_column, hgvs_nt_column, hgvs_pro_column, @@ -341,6 +342,9 @@ def validate_column_names(df: pd.DataFrame, kind: str, is_base_editor: bool) -> if len(columns) != len(set(columns)): raise ValidationError("duplicate column names are not allowed (this check is case insensitive)") + if accession_column in columns: + raise ValidationError(f"'{accession_column}' is a reserved column name (this check is case insensitive)") + if is_base_editor: msg = "dataframes for base editor data must also define the '{0}' column" if guide_sequence_column not in columns: diff --git a/tests/lib/csv/test_columns.py b/tests/lib/csv/test_columns.py index 595a60343..196984bc0 100644 --- a/tests/lib/csv/test_columns.py +++ b/tests/lib/csv/test_columns.py @@ -101,6 +101,18 @@ def test_an_absent_column_does_not_disturb_its_neighbours(self): "count2": "5", } + def test_investigator_column_named_accession_is_namespaced_even_when_unnamespaced(self): + """A score column named `accession` must not collide with core's own `accession` column. + + Headers (``assemble_csv_headers``) and row keys (here) share the same ``_format_column_key`` + decision, so asserting it here is enough to cover both. + """ + variant = MockVariant(urn="urn:mavedb:00000001-a-1#1", data={"score_data": {"accession": "sample-42"}}) + + row = variant_to_csv_row(variant, {"core": ["accession"], "scores": ["accession"]}, namespaced=False) + + assert row == {"accession": "urn:mavedb:00000001-a-1#1", "scores.accession": "sample-42"} + # --------------------------------------------------------------------------- # TestVariantToCsvRowUnrecognizedKey @@ -416,6 +428,24 @@ def test_plan_csv_columns_reference_hgvs_namespace_populates_columns(): False, ["accession", "clinvar.2024_01.clinical_significance"], ), + # An investigator column named `accession` collides with the core column of the same name, so it + # is forced under its namespace even though un-namespaced output would otherwise strip the prefix. + ( + {"core": ["accession", "hgvs_nt"], "scores": ["score", "accession"]}, + False, + ["accession", "hgvs_nt", "score", "scores.accession"], + ), + ( + {"core": ["accession"], "counts": ["count1", "accession"]}, + False, + ["accession", "count1", "counts.accession"], + ), + # `scores_custom` emits under `scores`, same as its non-colliding columns would. + ( + {"core": ["accession"], "scores_custom": ["accession"]}, + False, + ["accession", "scores.accession"], + ), # Mixed: respects insertion order ( { diff --git a/tests/lib/csv/test_variant.py b/tests/lib/csv/test_variant.py index a2e30c313..4929e23b8 100644 --- a/tests/lib/csv/test_variant.py +++ b/tests/lib/csv/test_variant.py @@ -22,6 +22,7 @@ available_variant_csv_namespaces, get_variant_csv, ) +from mavedb.lib.exceptions import NonexistentIdentifierError from mavedb.lib.mondo import get_generic_disease_term from mavedb.lib.permissions.principal import Principal from mavedb.lib.permissions.score_calibration import ScoreCalibrationViewer @@ -302,6 +303,18 @@ def test_unknown_urn_raises(self, session, setup_lib_db_with_mapped_variant): with pytest.raises(ValueError, match="not found"): get_variant_csv(session, "urn:mavedb:00000001-a-1#999") + def test_unknown_urn_raises_nonexistent_identifier_error(self, session, setup_lib_db_with_mapped_variant): + """Raised as ``NonexistentIdentifierError`` specifically, which the app maps to a 404 response + rather than letting a deleted-mid-request variant fall through to a 500.""" + with pytest.raises(NonexistentIdentifierError, match="not found"): + get_variant_csv(session, "urn:mavedb:00000001-a-1#999") + + def test_available_namespaces_unknown_urn_raises_nonexistent_identifier_error( + self, session, setup_lib_db_with_mapped_variant + ): + with pytest.raises(NonexistentIdentifierError, match="not found"): + available_variant_csv_namespaces(session, "urn:mavedb:00000001-a-1#999") + def test_no_calibration_yields_no_calibration_columns(self, session, setup_lib_db_with_mapped_variant): variant = setup_lib_db_with_mapped_variant.variant diff --git a/tests/validation/dataframe/test_dataframe.py b/tests/validation/dataframe/test_dataframe.py index daf3fd637..70bc4a3a6 100644 --- a/tests/validation/dataframe/test_dataframe.py +++ b/tests/validation/dataframe/test_dataframe.py @@ -6,6 +6,7 @@ import pytest from mavedb.lib.validation.constants.general import ( + accession_column, guide_sequence_column, hgvs_nt_column, hgvs_pro_column, @@ -387,6 +388,32 @@ def test_duplicate_data_column_names_counts(self): is_base_editor=False, ) + def test_column_named_accession_is_rejected_scores(self): + with self.assertRaises(ValidationError): + validate_column_names( + self.dataframe.rename(columns={"extra": accession_column}), kind="scores", is_base_editor=False + ) + + def test_column_named_accession_is_rejected_counts(self): + with self.assertRaises(ValidationError): + validate_column_names( + self.dataframe.drop([required_score_column], axis=1).rename(columns={"extra": accession_column}), + kind="counts", + is_base_editor=False, + ) + + def test_column_named_accession_is_rejected_case_insensitively(self): + with self.assertRaises(ValidationError): + validate_column_names( + self.dataframe.rename(columns={"extra": accession_column.upper()}), + kind="scores", + is_base_editor=False, + ) + + def test_hgvs_columns_are_still_accepted_alongside_accession_rejection(self): + """The accession rejection must not collide with the HGVS columns, which are always present.""" + validate_column_names(self.dataframe, kind="scores", is_base_editor=False) + # Written without @pytest.mark.parametrize. See: https://pytest.org/en/7.4.x/how-to/unittest.html#pytest-features-in-unittest-testcase-subclasses def test_invalid_column_names_scores(self): invalid_values = [None, np.nan, "", " "] From 561fdb27a090aefe0f35ae84561f6db12c75cf42 Mon Sep 17 00:00:00 2001 From: Estelle Da Date: Fri, 9 Oct 2026 15:57:58 +1100 Subject: [PATCH 2/8] Solve no module fastapi problem. No idea why merge doesn't solve this one. --- tests/view_models/test_allele_measurement.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/view_models/test_allele_measurement.py b/tests/view_models/test_allele_measurement.py index b74ebfeb6..48993ee35 100644 --- a/tests/view_models/test_allele_measurement.py +++ b/tests/view_models/test_allele_measurement.py @@ -1,6 +1,10 @@ +# ruff: noqa: E402 + import pytest from pydantic import ValidationError +fastapi = pytest.importorskip("fastapi") + from mavedb.view_models.allele_measurement import AlleleMeasurement from tests.helpers.constants import TEST_SAVED_FUNCTIONAL_RANGE_ABNORMAL, TEST_SAVED_FUNCTIONAL_RANGE_NORMAL From 75f0dfbc840cb447668a8ee91633c9f2495e8405 Mon Sep 17 00:00:00 2001 From: Estelle Da Date: Fri, 9 Oct 2026 16:09:44 +1100 Subject: [PATCH 3/8] Try to solve the psycopg2 problem. --- tests/lib/test_urns.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/lib/test_urns.py b/tests/lib/test_urns.py index 600c723b5..314c4fe61 100644 --- a/tests/lib/test_urns.py +++ b/tests/lib/test_urns.py @@ -1,3 +1,5 @@ +import importlib.util + import pytest from sqlalchemy import text from sqlalchemy.orm import sessionmaker @@ -101,6 +103,9 @@ def test_urns_without_a_variant_suffix_sort_last_without_raising(self, urn): @pytest.mark.integration +@pytest.mark.skipif( + importlib.util.find_spec("psycopg2") is None, reason="requires psycopg2 for a live database session" +) class TestLockUrnAssignment: @staticmethod def _other_session_can_take_lock(session) -> bool: From 7690565fe1a0b0504f203fffef06ed0d69ec275c Mon Sep 17 00:00:00 2001 From: Estelle Da Date: Mon, 12 Oct 2026 17:02:02 +1100 Subject: [PATCH 4/8] No idea why solved the conflicts in GitHub deleted some original code. --- tests/view_models/test_allele_measurement.py | 30 +++++++++++++++----- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/tests/view_models/test_allele_measurement.py b/tests/view_models/test_allele_measurement.py index 17a7face3..5346f988f 100644 --- a/tests/view_models/test_allele_measurement.py +++ b/tests/view_models/test_allele_measurement.py @@ -4,7 +4,7 @@ fastapi = pytest.importorskip("fastapi") from mavedb.view_models.allele_measurement import AlleleMeasurement -from tests.helpers.constants import TEST_SAVED_FUNCTIONAL_RANGE_ABNORMAL +from tests.helpers.constants import TEST_SAVED_FUNCTIONAL_RANGE_ABNORMAL, TEST_SAVED_FUNCTIONAL_RANGE_NORMAL MEASUREMENT = { "variantUrn": "urn:mavedb:00000001-a-1#1", @@ -15,10 +15,26 @@ } -def test_allele_measurement_classification_defaults_to_clinical(): - measurement = AlleleMeasurement.model_validate( - {**MEASUREMENT, "classification": TEST_SAVED_FUNCTIONAL_RANGE_ABNORMAL} - ) +@pytest.mark.parametrize( + "classifications", + [ + {}, + {"preferredClassification": TEST_SAVED_FUNCTIONAL_RANGE_NORMAL}, + {"researchUseOnlyClassification": TEST_SAVED_FUNCTIONAL_RANGE_ABNORMAL}, + ], +) +def test_allele_measurement_accepts_at_most_one_classification(classifications): + measurement = AlleleMeasurement.model_validate({**MEASUREMENT, **classifications}) - assert measurement.classification is not None - assert not measurement.classification_is_research_use_only + assert (measurement.preferred_classification is None) or (measurement.research_use_only_classification is None) + + +def test_allele_measurement_rejects_both_classifications(): + with pytest.raises(ValidationError, match="researchUseOnlyClassification may only be set"): + AlleleMeasurement.model_validate( + { + **MEASUREMENT, + "preferredClassification": TEST_SAVED_FUNCTIONAL_RANGE_NORMAL, + "researchUseOnlyClassification": TEST_SAVED_FUNCTIONAL_RANGE_ABNORMAL, + } + ) From dd45ca27a9904d9d2f36f96a7f0e537cd58f6157 Mon Sep 17 00:00:00 2001 From: Estelle Da Date: Mon, 12 Oct 2026 17:08:04 +1100 Subject: [PATCH 5/8] Add missing code back. --- tests/view_models/test_allele_measurement.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/view_models/test_allele_measurement.py b/tests/view_models/test_allele_measurement.py index 5346f988f..48993ee35 100644 --- a/tests/view_models/test_allele_measurement.py +++ b/tests/view_models/test_allele_measurement.py @@ -1,3 +1,5 @@ +# ruff: noqa: E402 + import pytest from pydantic import ValidationError From 84e89aad6e128917ce08274fe70bf857c9c27245 Mon Sep 17 00:00:00 2001 From: Estelle Da Date: Mon, 12 Oct 2026 18:38:12 +1100 Subject: [PATCH 6/8] Based on the merged new part, modified some related test functions. --- tests/view_models/test_allele_measurement.py | 31 +++++--------------- 1 file changed, 7 insertions(+), 24 deletions(-) diff --git a/tests/view_models/test_allele_measurement.py b/tests/view_models/test_allele_measurement.py index 48993ee35..7d27e4aa0 100644 --- a/tests/view_models/test_allele_measurement.py +++ b/tests/view_models/test_allele_measurement.py @@ -1,12 +1,11 @@ # ruff: noqa: E402 import pytest -from pydantic import ValidationError fastapi = pytest.importorskip("fastapi") from mavedb.view_models.allele_measurement import AlleleMeasurement -from tests.helpers.constants import TEST_SAVED_FUNCTIONAL_RANGE_ABNORMAL, TEST_SAVED_FUNCTIONAL_RANGE_NORMAL +from tests.helpers.constants import TEST_SAVED_FUNCTIONAL_RANGE_ABNORMAL MEASUREMENT = { "variantUrn": "urn:mavedb:00000001-a-1#1", @@ -17,26 +16,10 @@ } -@pytest.mark.parametrize( - "classifications", - [ - {}, - {"preferredClassification": TEST_SAVED_FUNCTIONAL_RANGE_NORMAL}, - {"researchUseOnlyClassification": TEST_SAVED_FUNCTIONAL_RANGE_ABNORMAL}, - ], -) -def test_allele_measurement_accepts_at_most_one_classification(classifications): - measurement = AlleleMeasurement.model_validate({**MEASUREMENT, **classifications}) +def test_allele_measurement_classification_defaults_to_clinical(): + measurement = AlleleMeasurement.model_validate( + {**MEASUREMENT, "classification": TEST_SAVED_FUNCTIONAL_RANGE_ABNORMAL} + ) - assert (measurement.preferred_classification is None) or (measurement.research_use_only_classification is None) - - -def test_allele_measurement_rejects_both_classifications(): - with pytest.raises(ValidationError, match="researchUseOnlyClassification may only be set"): - AlleleMeasurement.model_validate( - { - **MEASUREMENT, - "preferredClassification": TEST_SAVED_FUNCTIONAL_RANGE_NORMAL, - "researchUseOnlyClassification": TEST_SAVED_FUNCTIONAL_RANGE_ABNORMAL, - } - ) + assert measurement.classification is not None + assert not measurement.classification_is_research_use_only From cb28522f6639226c8099e7c75d74d510ede59dfd Mon Sep 17 00:00:00 2001 From: Estelle Da Date: Thu, 22 Oct 2026 14:52:18 +1100 Subject: [PATCH 7/8] Add two tests and remove unnecessary import. --- tests/routers/test_score_set.py | 113 ++++++++++++++++++++++++++++++++ 1 file changed, 113 insertions(+) diff --git a/tests/routers/test_score_set.py b/tests/routers/test_score_set.py index fae869f33..ac839cd9a 100644 --- a/tests/routers/test_score_set.py +++ b/tests/routers/test_score_set.py @@ -24,6 +24,9 @@ from mavedb.lib.annotation.exceptions import MappingDataDoesntExistException from mavedb.lib.exceptions import NonexistentOrcidUserError from mavedb.lib.score_distribution import summarize_scores +from mavedb.lib.score_sets import columns_for_dataset, create_variants, create_variants_data, csv_data_to_df +from mavedb.lib.validation.dataframe.dataframe import validate_and_standardize_dataframe_pair +from mavedb.lib.validation.exceptions import ValidationError from mavedb.lib.validation.urn_re import MAVEDB_EXPERIMENT_URN_RE, MAVEDB_SCORE_SET_URN_RE, MAVEDB_TMP_URN_RE from mavedb.models.annotation_event import AnnotationEvent from mavedb.models.clinvar_allele_link import ClinvarAlleleLink @@ -44,6 +47,7 @@ from mavedb.routers.score_sets import _annotation_stream_record from mavedb.view_models.orcid import OrcidUser from mavedb.view_models.score_set import ScoreSet, ScoreSetCreate +from mavedb.view_models.score_set_dataset_columns import DatasetColumnsCreate from tests.helpers.constants import ( EXTRA_LICENSE, EXTRA_USER, @@ -1753,6 +1757,42 @@ def test_upload_score_set_variant_data_returns_500_and_resets_processing_state_w assert pipelines == [] +def test_upload_score_set_variant_data_with_accession_column_is_accepted_but_fails_validation( + session, data_provider, client, setup_router_db, mock_s3_client, tmp_path +): + """The router only parses the upload and enqueues a job (see `upload_score_set_variant_data`); the + real column-name validation runs later, in the worker job that creates variants. This checks both + halves of the fix: the HTTP boundary accepts a column named `accession` rather than 500ing, and the + validation the worker would run on this exact data rejects it with a typed, user-facing error instead + of letting it through or crashing. + """ + experiment = create_experiment(client) + score_set = create_seq_score_set(client, experiment["urn"]) + + scores_csv_path = tmp_path / "scores_with_accession_column.csv" + scores_csv_path.write_text("hgvs_nt,hgvs_pro,score,accession\nc.1A>T,p.Thr1Ser,0.3,sample-1\n") + + with ( + open(scores_csv_path, "rb") as scores_file, + patch.object(arq.ArqRedis, "enqueue_job", return_value=None) as worker_queue, + patch.object(mock_s3_client, "upload_fileobj", return_value=None), + ): + response = client.post( + f"/api/v1/score-sets/{score_set['urn']}/variants/data", + files={"scores_file": (scores_csv_path.name, scores_file, "text/csv")}, + ) + worker_queue.assert_called_once() + + assert response.status_code == 200 + + item = session.scalars(select(ScoreSetDbModel).where(ScoreSetDbModel.urn == score_set["urn"])).one() + with open(scores_csv_path, "rb") as scores_file: + score_df = csv_data_to_df(scores_file) + + with pytest.raises(ValidationError, match="'accession' is a reserved column name"): + validate_and_standardize_dataframe_pair(score_df, None, None, None, item.target_genes, data_provider) + + def test_upload_score_set_variant_data_deletes_s3_files_and_pipeline_when_enqueue_job_fails( session, client, setup_router_db, data_files, mock_s3_client ): @@ -4089,6 +4129,79 @@ def test_download_counts_file(session, data_provider, client, setup_router_db, d assert "hgvs_splice" not in columns +def test_download_scores_and_counts_with_accession_dataset_column_returns_200( + session, data_provider, client, setup_router_db, tmp_path +): + """A score set whose score/count data already has a column literally named `accession` (data ingested + before uploads rejected that name) must still download rather than 500ing on the header collision with + the core `accession` column; the investigator's column is namespaced instead (`scores.accession` / + `counts.accession`). New uploads can no longer acquire such a column (see the upload-side test above), + so this models a pre-existing score set directly rather than going through the upload endpoint. + """ + experiment = create_experiment(client) + score_set = create_seq_score_set(client, experiment["urn"]) + + scores_csv_path = tmp_path / "scores_with_accession_column.csv" + scores_csv_path.write_text( + "hgvs_nt,hgvs_pro,score,accession\n" + "c.1A>T,p.Thr1Ser,0.3,sample-1\n" + "c.2C>T,p.Thr1Met,1.0,sample-2\n" + "c.6T>A,p.Phe2Leu,-1.65,sample-3\n" + ) + # A counts dataframe's non-hgvs columns must all be numeric (`validate_data_column`, force_numeric=True + # for kind="counts"), so unlike the scores file above, `accession` here has to be numeric too. + counts_csv_path = tmp_path / "counts_with_accession_column.csv" + counts_csv_path.write_text( + "hgvs_nt,hgvs_pro,count1,accession\n" + "c.1A>T,p.Thr1Ser,10,1\n" + "c.2C>T,p.Thr1Met,8,2\n" + "c.6T>A,p.Phe2Leu,90,3\n" + ) + with open(scores_csv_path, "rb") as scores_file: + scores_df = csv_data_to_df(scores_file) + with open(counts_csv_path, "rb") as counts_file: + counts_df = csv_data_to_df(counts_file) + + item = session.scalars(select(ScoreSetDbModel).where(ScoreSetDbModel.urn == score_set["urn"])).one() + + # Bypasses only the reserved-`accession`-column check, to model data that predates it; everything else + # runs through the real standardization pipeline. + with patch("mavedb.lib.validation.dataframe.dataframe.validate_column_names", return_value=None): + scores, counts, score_columns_metadata, count_columns_metadata = validate_and_standardize_dataframe_pair( + scores_df, counts_df, None, None, item.target_genes, data_provider + ) + + variants = create_variants_data(scores, counts, None) + num_variants = create_variants(session, item, variants) + assert num_variants == len(scores) + + item.processing_state = ProcessingState.success + item.dataset_columns = DatasetColumnsCreate( + score_columns=columns_for_dataset(scores), + count_columns=columns_for_dataset(counts), + score_columns_metadata=score_columns_metadata if score_columns_metadata is not None else {}, + count_columns_metadata=count_columns_metadata if count_columns_metadata is not None else {}, + ).model_dump() + session.add(item) + session.commit() + + with patch.object(arq.ArqRedis, "enqueue_job", return_value=None) as worker_queue: + published_score_set = publish_score_set(client, score_set["urn"]) + worker_queue.assert_called_once() + + scores_response = client.get(f"/api/v1/score-sets/{published_score_set['urn']}/scores") + assert scores_response.status_code == 200 + scores_header = scores_response.text.splitlines()[0].split(",") + assert "accession" in scores_header + assert "scores.accession" in scores_header + + counts_response = client.get(f"/api/v1/score-sets/{published_score_set['urn']}/counts") + assert counts_response.status_code == 200 + counts_header = counts_response.text.splitlines()[0].split(",") + assert "accession" in counts_header + assert "counts.accession" in counts_header + + @pytest.mark.parametrize("path", ["scores", "counts", "variants/data"]) def test_csv_routes_raise_the_statement_timeout(session, data_provider, client, setup_router_db, data_files, path): experiment = create_experiment(client) From f03e03810e8d8b113fb0384279be47d90d6b6b08 Mon Sep 17 00:00:00 2001 From: Estelle Da Date: Thu, 22 Oct 2026 15:08:16 +1100 Subject: [PATCH 8/8] Fix formatting of CSV fixtures in new accession-column router tests bin/check's ruff format disagreed with the local pinned version on how to lay out the multi-line CSV string literals. Co-Authored-By: Claude Sonnet 5 --- tests/routers/test_score_set.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/tests/routers/test_score_set.py b/tests/routers/test_score_set.py index ac839cd9a..07a870e13 100644 --- a/tests/routers/test_score_set.py +++ b/tests/routers/test_score_set.py @@ -4143,19 +4143,13 @@ def test_download_scores_and_counts_with_accession_dataset_column_returns_200( scores_csv_path = tmp_path / "scores_with_accession_column.csv" scores_csv_path.write_text( - "hgvs_nt,hgvs_pro,score,accession\n" - "c.1A>T,p.Thr1Ser,0.3,sample-1\n" - "c.2C>T,p.Thr1Met,1.0,sample-2\n" - "c.6T>A,p.Phe2Leu,-1.65,sample-3\n" + "hgvs_nt,hgvs_pro,score,accession\nc.1A>T,p.Thr1Ser,0.3,sample-1\nc.2C>T,p.Thr1Met,1.0,sample-2\nc.6T>A,p.Phe2Leu,-1.65,sample-3\n" ) # A counts dataframe's non-hgvs columns must all be numeric (`validate_data_column`, force_numeric=True # for kind="counts"), so unlike the scores file above, `accession` here has to be numeric too. counts_csv_path = tmp_path / "counts_with_accession_column.csv" counts_csv_path.write_text( - "hgvs_nt,hgvs_pro,count1,accession\n" - "c.1A>T,p.Thr1Ser,10,1\n" - "c.2C>T,p.Thr1Met,8,2\n" - "c.6T>A,p.Phe2Leu,90,3\n" + "hgvs_nt,hgvs_pro,count1,accession\nc.1A>T,p.Thr1Ser,10,1\nc.2C>T,p.Thr1Met,8,2\nc.6T>A,p.Phe2Leu,90,3\n" ) with open(scores_csv_path, "rb") as scores_file: scores_df = csv_data_to_df(scores_file)