diff --git a/.github/workflows/branch.yml b/.github/workflows/branch.yml
index b40942e..385d297 100644
--- a/.github/workflows/branch.yml
+++ b/.github/workflows/branch.yml
@@ -20,7 +20,7 @@ jobs:
# NOTE - this doesn't currently work if the PR is coming from a fork, due to limitations in GitHub actions secrets
- name: Post PR comment
if: failure()
- uses: mshick/add-pr-comment@ffd016c7e151d97d69d21a843022fd4cd5b96fe5 # v3.9.0
+ uses: mshick/add-pr-comment@8e4927817251f1ff60c001f04568532b38e0b4a0 # v3.11.0
with:
message: |
## This PR is against the `main` branch :x:
diff --git a/pyproject.toml b/pyproject.toml
index 18fe397..2ab98db 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -9,7 +9,7 @@ readme = "README.md"
[tool.poetry.dependencies]
python = "^3.11"
# use `rev` to pin to a specific hash, or tag, from the git repo
-labretriever = {git = "https://github.com/cmatKhan/labretriever.git", rev = "72bbf3b"}
+labretriever = {git = "https://github.com/cmatKhan/labretriever.git", rev = "ecaa7e3"}
shiny = "^1.4.0"
shinywidgets = "^0.7.1"
upsetjs-jupyter-widget = "^1.9.0"
diff --git a/tests/unit/test_sample_conditions.py b/tests/unit/test_sample_conditions.py
new file mode 100644
index 0000000..c0b194d
--- /dev/null
+++ b/tests/unit/test_sample_conditions.py
@@ -0,0 +1,126 @@
+"""Unit tests for ``tfbpshiny.utils.sample_conditions``."""
+
+from __future__ import annotations
+
+from typing import Any
+
+import pandas as pd
+import pytest
+
+from tfbpshiny.utils.sample_conditions import (
+ build_condition_label,
+ fetch_sample_condition_map,
+)
+
+# ---------- build_condition_label ----------
+
+
+@pytest.mark.parametrize(
+ "values,expected",
+ [
+ (["YPD"], "YPD"),
+ (["ZEV", "P", "45"], "ZEV / P / 45"),
+ (["ZEV", None, "45"], "ZEV / 45"),
+ ([None, None], ""),
+ ([], ""),
+ ([" spaced "], "spaced"),
+ (["", " ", "YPD"], "YPD"),
+ (["nan", "NaN", "YPD"], "YPD"),
+ ([float("nan"), "YPD"], "YPD"),
+ ([1, 2.5, "x"], "1 / 2.5 / x"),
+ ],
+)
+def test_build_condition_label(values: list[Any], expected: str) -> None:
+ assert build_condition_label(values) == expected
+
+
+# ---------- fetch_sample_condition_map ----------
+
+
+class _StubVdb:
+ """Minimal stub capturing the last-issued SQL and returning a canned DataFrame."""
+
+ def __init__(self, df: pd.DataFrame) -> None:
+ self._df = df
+ self.last_sql: str | None = None
+
+ def query(self, sql: str, **params: Any) -> pd.DataFrame:
+ self.last_sql = sql
+ return self._df
+
+
+def test_fetch_sample_condition_map_empty_cols_returns_empty() -> None:
+ vdb = _StubVdb(pd.DataFrame())
+ assert fetch_sample_condition_map(vdb, "anything", []) == {}
+ # No query should have been issued.
+ assert vdb.last_sql is None
+
+
+def test_fetch_sample_condition_map_single_column() -> None:
+ df = pd.DataFrame(
+ {
+ "sample_id": ["s1", "s2", "s3"],
+ "Experimental condition": ["YPD", "HEAT", None],
+ }
+ )
+ vdb = _StubVdb(df)
+ result = fetch_sample_condition_map(vdb, "harbison", ["Experimental condition"])
+
+ assert result == {"s1": "YPD", "s2": "HEAT"}
+ # s3 had an all-NULL label and must not be present.
+ assert "s3" not in result
+ # Column with a space is double-quoted in the SQL.
+ assert vdb.last_sql is not None
+ assert '"Experimental condition"' in vdb.last_sql
+ assert "FROM harbison_meta" in vdb.last_sql
+
+
+def test_fetch_sample_condition_map_multi_column_joined() -> None:
+ df = pd.DataFrame(
+ {
+ "sample_id": ["s1", "s2"],
+ "mechanism": ["ZEV", "GEV"],
+ "restriction": ["P", None],
+ }
+ )
+ vdb = _StubVdb(df)
+ result = fetch_sample_condition_map(vdb, "hackett", ["mechanism", "restriction"])
+
+ assert result == {"s1": "ZEV / P", "s2": "GEV"}
+
+
+def test_fetch_sample_condition_map_non_string_sample_id_coerced() -> None:
+ # sample_id comes back from DuckDB as int or bytes in some schemas.
+ df = pd.DataFrame({"sample_id": [1, 2], "cond": ["A", "B"]})
+ vdb = _StubVdb(df)
+ result = fetch_sample_condition_map(vdb, "ds", ["cond"])
+ assert result == {"1": "A", "2": "B"}
+
+
+@pytest.mark.parametrize(
+ "bad_name",
+ [
+ "foo; DROP TABLE users",
+ "foo-bar",
+ "foo bar",
+ "1foo",
+ "",
+ 'foo"; SELECT',
+ ],
+)
+def test_fetch_sample_condition_map_rejects_unsafe_db_name(bad_name: str) -> None:
+ vdb = _StubVdb(pd.DataFrame({"sample_id": ["s1"], "cond": ["A"]}))
+ with pytest.raises(ValueError, match="safe SQL identifier"):
+ fetch_sample_condition_map(vdb, bad_name, ["cond"])
+
+
+def test_fetch_sample_condition_map_escapes_embedded_quotes_in_cols() -> None:
+ # Column name with an embedded double-quote must be escaped as "" per
+ # DuckDB identifier rules; otherwise the SQL would break out of quoting.
+ df = pd.DataFrame({"sample_id": ["s1"], 'weird"name': ["A"]})
+ vdb = _StubVdb(df)
+ result = fetch_sample_condition_map(vdb, "ds", ['weird"name'])
+ assert result == {"s1": "A"}
+ assert vdb.last_sql is not None
+ # The emitted identifier is "weird""name" — one escaped embedded quote.
+ assert '"weird""name"' in vdb.last_sql
diff --git a/tfbpshiny/app.py b/tfbpshiny/app.py
index 60e4eed..ae5ca4e 100644
--- a/tfbpshiny/app.py
+++ b/tfbpshiny/app.py
@@ -137,6 +137,7 @@ def app_server(input: Any, output: Any, session: Any) -> None:
col_preference=col_preference,
dataset_filters=dataset_filters,
vdb=vdb,
+ app_datasets=app_datasets,
logger=logger,
)
@@ -154,6 +155,7 @@ def app_server(input: Any, output: Any, session: Any) -> None:
col_preference=col_preference_p,
dataset_filters=dataset_filters,
vdb=vdb,
+ app_datasets=app_datasets,
logger=logger,
)
diff --git a/tfbpshiny/modules/binding/page_test.py b/tfbpshiny/modules/binding/page_test.py
index d6b784c..c55b1e6 100644
--- a/tfbpshiny/modules/binding/page_test.py
+++ b/tfbpshiny/modules/binding/page_test.py
@@ -24,6 +24,7 @@
binding_workspace_server,
)
from tfbpshiny.modules.binding.ui import binding_sidebar_ui, binding_workspace_ui
+from tfbpshiny.utils.vdb_init import AppDatasets
logger = logging.getLogger("shiny")
@@ -157,6 +158,7 @@ def active_binding_datasets() -> list[str]:
col_preference=col_preference,
dataset_filters=_dataset_filters,
vdb=vdb,
+ app_datasets=AppDatasets(condition_cols={}, upstream_cols={}),
logger=logger,
)
diff --git a/tfbpshiny/modules/binding/server/workspace.py b/tfbpshiny/modules/binding/server/workspace.py
index fcb731b..c31f727 100644
--- a/tfbpshiny/modules/binding/server/workspace.py
+++ b/tfbpshiny/modules/binding/server/workspace.py
@@ -2,6 +2,7 @@
import itertools
from collections.abc import Callable
+from html import escape
from logging import Logger
from typing import Any, Literal
@@ -16,7 +17,8 @@
get_measurement_column,
regulator_scatter_sql,
)
-from tfbpshiny.utils.vdb_init import get_regulator_display_name
+from tfbpshiny.utils.sample_conditions import fetch_sample_condition_map
+from tfbpshiny.utils.vdb_init import AppDatasets, get_regulator_display_name
@module.server
@@ -29,6 +31,7 @@ def binding_workspace_server(
col_preference: Callable[[], str],
dataset_filters: reactive.Value[dict[str, Any]],
vdb: VirtualDB,
+ app_datasets: AppDatasets,
logger: Logger,
) -> None:
"""
@@ -46,6 +49,35 @@ def binding_workspace_server(
zip(_reg_df["regulator_locus_tag"], _reg_df["display_name"])
)
+ @reactive.calc
+ def _condition_maps() -> dict[str, dict[str, str]]:
+ """
+ ``{db_name: {sample_id: label}}`` for each active dataset that has
+ experimental_condition columns.
+
+ Used to annotate tooltips on the selected-regulator overlay in the
+ distribution plot so the user can distinguish multiple samples of the
+ same regulator. Datasets without ``condition_cols`` are omitted from
+ the outer dict, causing the tooltip to skip their side.
+
+ :trigger active_binding_datasets: re-runs when the user toggles a
+ binding dataset on or off.
+ :returns: Outer dict keyed by db_name; inner dict maps sample_id to
+ the joined condition label.
+
+ """
+ out: dict[str, dict[str, str]] = {}
+ for db in active_binding_datasets():
+ cols = app_datasets.condition_cols.get(db, [])
+ if not cols:
+ continue
+ try:
+ out[db] = fetch_sample_condition_map(vdb, db, cols)
+ except Exception:
+ logger.exception("Failed to fetch condition map for %s", db)
+ out[db] = {}
+ return out
+
@reactive.calc
def _pairs() -> list[tuple[str, str]]:
"""
@@ -169,6 +201,8 @@ def distributions_plot() -> ui.Tag:
except Exception:
selected_reg = ""
+ cond_maps = _condition_maps()
+
# Build a single combined box trace using x as the category axis.
# Each point's x value is the pair label; Plotly groups points under
# each category and draws one box per unique x value.
@@ -185,10 +219,15 @@ def distributions_plot() -> ui.Tag:
label_a = display_names.get(db_a, db_a)
label_b = display_names.get(db_b, db_b)
pair_label = f"{label_a}
vs
{label_b}"
+ cond_a = cond_maps.get(db_a, {})
+ cond_b = cond_maps.get(db_b, {})
if not df.empty:
df_clean = df.dropna(subset=["correlation"])
- for tag, corr in zip(
- df_clean["regulator_locus_tag"], df_clean["correlation"]
+ for tag, corr, sample_a, sample_b in zip(
+ df_clean["regulator_locus_tag"],
+ df_clean["correlation"],
+ df_clean["db_a_id"],
+ df_clean["db_b_id"],
):
display = sym_map.get(tag, tag)
all_x.append(pair_label)
@@ -198,7 +237,24 @@ def distributions_plot() -> ui.Tag:
if tag == selected_reg:
sel_x.append(pair_label)
sel_y.append(corr)
- sel_hover.append(display)
+ # Per-dot hover: regulator + r + one condition line per
+ # dataset that has a non-empty label for this sample.
+ # All DB-sourced strings are HTML-escaped before being
+ # joined with the
separators because Plotly renders
+ # hovertext as HTML (stored-XSS sink if any researcher-
+ # uploaded metadata ever contained markup).
+ hover_lines = [escape(display), f"r = {corr:.3f}"]
+ label_sample_a = cond_a.get(str(sample_a), "")
+ if label_sample_a:
+ hover_lines.append(
+ f"{escape(label_a)}: {escape(label_sample_a)}"
+ )
+ label_sample_b = cond_b.get(str(sample_b), "")
+ if label_sample_b:
+ hover_lines.append(
+ f"{escape(label_b)}: {escape(label_sample_b)}"
+ )
+ sel_hover.append("
".join(hover_lines))
sel_tags.append(tag)
fig.add_trace(
@@ -224,9 +280,9 @@ def distributions_plot() -> ui.Tag:
x=sel_x,
y=sel_y,
mode="markers",
- text=sel_hover,
+ hovertext=sel_hover,
customdata=sel_tags,
- hovertemplate="%{text}
r = %{y:.3f}",
+ hovertemplate="%{hovertext}",
marker=dict(size=10, color="black", symbol="circle"),
showlegend=False,
)
diff --git a/tfbpshiny/modules/perturbation/page_test.py b/tfbpshiny/modules/perturbation/page_test.py
index af8f531..ccce01d 100644
--- a/tfbpshiny/modules/perturbation/page_test.py
+++ b/tfbpshiny/modules/perturbation/page_test.py
@@ -28,6 +28,7 @@
perturbation_sidebar_ui,
perturbation_workspace_ui,
)
+from tfbpshiny.utils.vdb_init import AppDatasets
logger = logging.getLogger("shiny")
@@ -165,6 +166,7 @@ def active_perturbation_datasets() -> list[str]:
col_preference=col_preference,
dataset_filters=_dataset_filters,
vdb=vdb,
+ app_datasets=AppDatasets(condition_cols={}, upstream_cols={}),
logger=logger,
)
diff --git a/tfbpshiny/modules/perturbation/server/workspace.py b/tfbpshiny/modules/perturbation/server/workspace.py
index d799979..927c984 100644
--- a/tfbpshiny/modules/perturbation/server/workspace.py
+++ b/tfbpshiny/modules/perturbation/server/workspace.py
@@ -2,6 +2,7 @@
import itertools
from collections.abc import Callable
+from html import escape
from logging import Logger
from typing import Any, Literal
@@ -16,7 +17,8 @@
get_measurement_column,
regulator_scatter_sql,
)
-from tfbpshiny.utils.vdb_init import get_regulator_display_name
+from tfbpshiny.utils.sample_conditions import fetch_sample_condition_map
+from tfbpshiny.utils.vdb_init import AppDatasets, get_regulator_display_name
@module.server
@@ -29,6 +31,7 @@ def perturbation_workspace_server(
col_preference: Callable[[], str],
dataset_filters: reactive.Value[dict[str, Any]],
vdb: VirtualDB,
+ app_datasets: AppDatasets,
logger: Logger,
) -> None:
"""
@@ -46,6 +49,35 @@ def perturbation_workspace_server(
zip(_reg_df["regulator_locus_tag"], _reg_df["display_name"])
)
+ @reactive.calc
+ def _condition_maps() -> dict[str, dict[str, str]]:
+ """
+ ``{db_name: {sample_id: label}}`` for each active dataset that has
+ experimental_condition columns.
+
+ Used to annotate tooltips on the selected-regulator overlay in the
+ distribution plot so the user can distinguish multiple samples of the
+ same regulator. Datasets without ``condition_cols`` are omitted from
+ the outer dict, causing the tooltip to skip their side.
+
+ :trigger active_perturbation_datasets: re-runs when the user toggles
+ a perturbation dataset on or off.
+ :returns: Outer dict keyed by db_name; inner dict maps sample_id to
+ the joined condition label.
+
+ """
+ out: dict[str, dict[str, str]] = {}
+ for db in active_perturbation_datasets():
+ cols = app_datasets.condition_cols.get(db, [])
+ if not cols:
+ continue
+ try:
+ out[db] = fetch_sample_condition_map(vdb, db, cols)
+ except Exception:
+ logger.exception("Failed to fetch condition map for %s", db)
+ out[db] = {}
+ return out
+
@reactive.calc
def _pairs() -> list[tuple[str, str]]:
"""
@@ -167,6 +199,8 @@ def distributions_plot() -> ui.Tag:
except Exception:
selected_reg = ""
+ cond_maps = _condition_maps()
+
# Build a single combined box trace using x as the category axis.
# Each point's x value is the pair label; Plotly groups points under
# each category and draws one box per unique x value.
@@ -183,10 +217,15 @@ def distributions_plot() -> ui.Tag:
label_a = display_names.get(db_a, db_a)
label_b = display_names.get(db_b, db_b)
pair_label = f"{label_a}
vs
{label_b}"
+ cond_a = cond_maps.get(db_a, {})
+ cond_b = cond_maps.get(db_b, {})
if not df.empty:
df_clean = df.dropna(subset=["correlation"])
- for tag, corr in zip(
- df_clean["regulator_locus_tag"], df_clean["correlation"]
+ for tag, corr, sample_a, sample_b in zip(
+ df_clean["regulator_locus_tag"],
+ df_clean["correlation"],
+ df_clean["db_a_id"],
+ df_clean["db_b_id"],
):
display = sym_map.get(tag, tag)
all_x.append(pair_label)
@@ -196,7 +235,24 @@ def distributions_plot() -> ui.Tag:
if tag == selected_reg:
sel_x.append(pair_label)
sel_y.append(corr)
- sel_hover.append(display)
+ # Per-dot hover: regulator + r + one condition line per
+ # dataset that has a non-empty label for this sample.
+ # All DB-sourced strings are HTML-escaped before being
+ # joined with the
separators because Plotly renders
+ # hovertext as HTML (stored-XSS sink if any researcher-
+ # uploaded metadata ever contained markup).
+ hover_lines = [escape(display), f"r = {corr:.3f}"]
+ label_sample_a = cond_a.get(str(sample_a), "")
+ if label_sample_a:
+ hover_lines.append(
+ f"{escape(label_a)}: {escape(label_sample_a)}"
+ )
+ label_sample_b = cond_b.get(str(sample_b), "")
+ if label_sample_b:
+ hover_lines.append(
+ f"{escape(label_b)}: {escape(label_sample_b)}"
+ )
+ sel_hover.append("
".join(hover_lines))
sel_tags.append(tag)
fig.add_trace(
@@ -222,9 +278,9 @@ def distributions_plot() -> ui.Tag:
x=sel_x,
y=sel_y,
mode="markers",
- text=sel_hover,
+ hovertext=sel_hover,
customdata=sel_tags,
- hovertemplate="%{text}
r = %{y:.3f}",
+ hovertemplate="%{hovertext}",
marker=dict(size=10, color="black", symbol="circle"),
showlegend=False,
)
diff --git a/tfbpshiny/modules/select_datasets/export.py b/tfbpshiny/modules/select_datasets/export.py
index 5d7a90e..9771f7f 100644
--- a/tfbpshiny/modules/select_datasets/export.py
+++ b/tfbpshiny/modules/select_datasets/export.py
@@ -140,9 +140,13 @@ def build_export_tarball(
:func:`_query_to_csv_bytes` so only one dataset's data is in memory at
once.
- Uses ``tarfile`` pipe mode (``w|gz``) for streaming writes into an
- in-memory ``BytesIO`` buffer — no temp files on disk. The full buffer
- is returned to the caller for chunked yielding.
+ Uses ``tarfile`` non-pipe mode (``w:gz``) with a seekable in-memory
+ ``BytesIO`` buffer — no temp files on disk. The full buffer is returned
+ to the caller for chunked yielding. Gzip compression is pinned to
+ ``compresslevel=1`` to favour export speed over archive size; the
+ absolute speedup and size penalty depend on the dataset mix (see
+ issue #242). ``compresslevel`` is not accepted in pipe mode (``w|gz``),
+ which is why non-pipe mode is used here.
A thread-safe DuckDB cursor is created via ``vdb._conn.cursor()`` so
this function can safely run in a worker thread while the main event
@@ -161,7 +165,7 @@ def build_export_tarball(
out = io.BytesIO()
try:
- with tarfile.open(mode="w|gz", fileobj=out) as tar:
+ with tarfile.open(mode="w:gz", fileobj=out, compresslevel=1) as tar:
for ds in datasets:
dir_name = _safe_dir_name(ds.display_name)
diff --git a/tfbpshiny/utils/sample_conditions.py b/tfbpshiny/utils/sample_conditions.py
new file mode 100644
index 0000000..8748b5e
--- /dev/null
+++ b/tfbpshiny/utils/sample_conditions.py
@@ -0,0 +1,97 @@
+"""
+Helpers for attaching experimental-condition labels to individual samples.
+
+The binding and perturbation distribution plots show one dot per sample pair. When
+a regulator has multiple samples in a dataset the plot produces multiple dots for
+that regulator, and the user needs a way to tell them apart in the tooltip. This
+module builds ``{sample_id: condition_label}`` lookups from a dataset's ``_meta``
+view so that the plot code can append human-readable condition text to each
+selected-overlay dot's hover string.
+
+``build_condition_label`` is a pure function and is the main unit-testable
+surface. ``fetch_sample_condition_map`` is a thin VirtualDB wrapper that assembles
+the SQL, runs it, and applies the label builder row-by-row.
+
+"""
+
+from __future__ import annotations
+
+import re
+from typing import Any
+
+from labretriever import VirtualDB
+
+
+def build_condition_label(values: list[Any]) -> str:
+ """
+ Combine one or more raw condition-column values into a single label.
+
+ Non-empty, non-NaN string representations are joined with ``" / "``. A
+ value of ``None``, a ``NaN`` float, or a string that is empty / whitespace
+ / the literal ``"nan"`` (case-insensitive) is dropped. When every value
+ drops out, an empty string is returned.
+
+ :param values: One or more raw values from a single meta-table row.
+ :returns: Display label, or ``""`` when nothing useful remains.
+
+ """
+ parts: list[str] = []
+ for v in values:
+ if v is None:
+ continue
+ # Catch float NaN without requiring a pandas/numpy import here.
+ if isinstance(v, float) and v != v:
+ continue
+ s = str(v).strip()
+ if not s or s.lower() == "nan":
+ continue
+ parts.append(s)
+ return " / ".join(parts)
+
+
+_SAFE_IDENT_RE = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$")
+
+
+def fetch_sample_condition_map(
+ vdb: VirtualDB, db_name: str, cols: list[str]
+) -> dict[str, str]:
+ """
+ Build a ``{sample_id: condition_label}`` map for one dataset.
+
+ Queries ``{db_name}_meta`` for ``sample_id`` plus the requested condition
+ columns and composes each row's label with :func:`build_condition_label`.
+ Samples whose composed label is empty are omitted.
+
+ Identifier safety: ``db_name`` must match a SQL-identifier pattern
+ (``[A-Za-z_][A-Za-z0-9_]*``), otherwise a ``ValueError`` is raised before
+ any SQL is built. Column names are double-quoted with embedded quotes
+ doubled (DuckDB's identifier-escape), so columns containing spaces are
+ fine but a column containing ``"`` cannot break out of the quoting.
+
+ :param vdb: VirtualDB instance.
+ :param db_name: Dataset name (the base name; ``_meta`` is appended).
+ Must be a valid SQL identifier.
+ :param cols: Condition column names, as taken from
+ ``AppDatasets.condition_cols[db_name]``. May include spaces — each
+ column is double-quoted in the generated SQL.
+ :returns: Mapping from ``sample_id`` to its joined condition label. Empty
+ when ``cols`` is empty.
+ :raises ValueError: If ``db_name`` is not a safe identifier.
+
+ """
+ if not cols:
+ return {}
+ if not _SAFE_IDENT_RE.match(db_name):
+ raise ValueError(f"db_name is not a safe SQL identifier: {db_name!r}")
+ quoted = ", ".join(f'"{c.replace(chr(34), chr(34) * 2)}"' for c in cols)
+ sql = f"SELECT sample_id, {quoted} FROM {db_name}_meta"
+ df = vdb.query(sql)
+ result: dict[str, str] = {}
+ for _, row in df.iterrows():
+ label = build_condition_label([row[c] for c in cols])
+ if label:
+ result[str(row["sample_id"])] = label
+ return result
+
+
+__all__ = ["build_condition_label", "fetch_sample_condition_map"]