Skip to content
17 changes: 16 additions & 1 deletion src/mavedb/lib/csv/columns.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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}"
Expand Down Expand Up @@ -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
Expand Down
9 changes: 7 additions & 2 deletions src/mavedb/lib/csv/specs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -141,7 +146,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),
Expand Down
9 changes: 5 additions & 4 deletions src/mavedb/lib/csv/variant.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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={},
Expand Down
1 change: 1 addition & 0 deletions src/mavedb/lib/validation/constants/general.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
4 changes: 4 additions & 0 deletions src/mavedb/lib/validation/dataframe/dataframe.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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:
Expand Down
30 changes: 30 additions & 0 deletions tests/lib/csv/test_columns.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
(
{
Expand Down
13 changes: 13 additions & 0 deletions tests/lib/csv/test_variant.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -315,6 +316,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

Expand Down
27 changes: 27 additions & 0 deletions tests/validation/dataframe/test_dataframe.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import pytest

from mavedb.lib.validation.constants.general import (
accession_column,
guide_sequence_column,
hgvs_nt_column,
hgvs_pro_column,
Expand Down Expand Up @@ -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, "", " "]
Expand Down
6 changes: 6 additions & 0 deletions tests/view_models/test_allele_measurement.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
# ruff: noqa: E402

import pytest

fastapi = pytest.importorskip("fastapi")

from mavedb.view_models.allele_measurement import AlleleMeasurement
from tests.helpers.constants import TEST_SAVED_FUNCTIONAL_RANGE_ABNORMAL

Expand Down
Loading