From 508e20f39c6c1f586c0c4465e163ad62518f19a5 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Oct 2026 06:10:58 +0000 Subject: [PATCH 1/2] Experimental design table with design.tsv and SDRF import StreamlitUI.experimental_design() shows a condition / biological replicate / fraction table for a workflow's selected input files, with import of OpenMS design files (one- and two-table) and SDRF, a design.tsv download, and notices for missing conditions, fraction gaps, uneven fractions and technical replicates. It saves the design as an OpenMS two-table design.tsv in the workflow directory, which TOPP tools take via -design. src/workflow/ExperimentalDesign.py holds the Streamlit-free reading, writing and checking, so execution() can use it in a queue worker. The example workflow shows it in a new tab. Also fixes input_widget losing a value restored from params.json: the initial-value argument was still passed on the first render, so on Streamlit < 1.50 the widget id changed once and the next rerun of the enclosing fragment reset the widget (a restored file selection came back empty and was saved as empty). The first render now writes the value into session state instead. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01D5Wo2FHPpi5a8X6GppNxEk --- CLAUDE.md | 2 + src/Workflow.py | 18 ++- src/workflow/ExperimentalDesign.py | 239 +++++++++++++++++++++++++++++ src/workflow/StreamlitUI.py | 131 ++++++++++++++-- tests/test_experimental_design.py | 110 +++++++++++++ tests/test_input_widget_seed.py | 45 ++++++ 6 files changed, 529 insertions(+), 16 deletions(-) create mode 100644 src/workflow/ExperimentalDesign.py create mode 100644 tests/test_experimental_design.py create mode 100644 tests/test_input_widget_seed.py diff --git a/CLAUDE.md b/CLAUDE.md index d599f15ab..ee89c96bf 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -134,6 +134,8 @@ Subclass `WorkflowManager` (see `src/Workflow.py`) and implement four methods: Each workflow gets four thin content pages calling `wf.show_file_upload_section()` / `show_parameter_section()` / `show_execution_section()` / `show_results_section()`. An optional fifth page calls `wf.show_results_download_section(exclude=[...])`, which offers every file under `/results/` singly or as a ZIP (pass `exclude` for caches that are not results). Decorate `configure()` and `results()` with `@st.fragment`. +**Experimental design**: `self.ui.experimental_design("mzML-files")` in `configure()` shows a condition / biological replicate / fraction table for the selected files (pass `reactive=True` to that `select_input_file` so the table follows the selection), with design.tsv and SDRF import. It saves `/experimental-design.tsv` in the OpenMS two-table format, which a TOPP tool takes as `-design`; `src/workflow/ExperimentalDesign.py` (Streamlit-free, so usable in `execution()`) reads, writes and checks it. + **`execution()` must be annotated `-> bool` and `return True` only when every step succeeded.** Both callers — `workflow_process()` locally and `tasks.execute_workflow()` in queue mode — log the `WORKFLOW FINISHED` marker only for a truthy return, and a missing marker is classified as an error. `run_topp()` and `run_python()` both return `False` on failure; gate every call on it, or a run whose third tool died is still reported as a success. The shipped example (`src/Workflow.py`) does exactly this — copy its structure *and* its return type. **Upgrading a fork:** queue mode previously wrote the marker unconditionally and returned a result dict from its exception handler, so *every* queued job was recorded successful and RQ's `FailedJobRegistry` was structurally empty. It now honours the return value and re-raises. A subclass still declared `execution() -> None` keeps working — `None` is accepted as success with a deprecation warning in the workflow log — but it reports nothing when a tool fails, so fix the signature when you rebase. diff --git a/src/Workflow.py b/src/Workflow.py index 02245c250..39614529e 100644 --- a/src/Workflow.py +++ b/src/Workflow.py @@ -28,12 +28,18 @@ def upload(self) -> None: @st.fragment def configure(self) -> None: - # Allow users to select mzML files for the analysis. - self.ui.select_input_file("mzML-files", multiple=True) + # Allow users to select mzML files for the analysis. reactive=True so + # the experimental design table follows the selection. + self.ui.select_input_file("mzML-files", multiple=True, reactive=True) # Create tabs for different analysis steps. t = st.tabs( - ["**Feature Detection**", "**Feature Linking**", "**Python Custom Tool**"] + [ + "**Feature Detection**", + "**Feature Linking**", + "**Python Custom Tool**", + "**Experimental Design**", + ] ) with t[0]: # Parameters for FeatureFinderMetabo TOPP tool. @@ -50,6 +56,12 @@ def configure(self) -> None: # Generate input widgets for a custom Python tool, located at src/python-tools. # Parameters are specified within the file in the DEFAULTS dictionary. self.ui.input_python("example") + with t[3]: + # Condition, replicate and fraction per selected file, saved as an + # OpenMS design.tsv in the workflow directory. Pass that file to a + # TOPP tool's -design (e.g. ProteomicsLFQ), or read it in + # execution() with src.workflow.ExperimentalDesign.load_design(). + self.ui.experimental_design("mzML-files") def execution(self) -> bool: # Return True only if every step succeeded. The caller diff --git a/src/workflow/ExperimentalDesign.py b/src/workflow/ExperimentalDesign.py new file mode 100644 index 000000000..aad1982bc --- /dev/null +++ b/src/workflow/ExperimentalDesign.py @@ -0,0 +1,239 @@ +"""Experimental design: which condition, biological replicate and fraction each MS file belongs to. + +The design is a table with one row per file and the columns ``file`` (the +file name), ``condition``, ``replicate`` (biological replicate, 1-based) and +``fraction`` (1-based). Two rows with the same condition, replicate and +fraction are technical replicates of one sample. + +It is read from and written to the OpenMS experimental design format +(``design.tsv``, which ProteomicsLFQ, ProteinQuantifier and MSstatsConverter +take via ``-design``), and read from SDRF. This module has no Streamlit +dependency, so ``execution()`` can call ``load_design()`` inside a queue +worker. +""" +import io +from pathlib import Path +from typing import List, Union + +import pandas as pd + +DESIGN_FILE = "experimental-design.tsv" +COLUMNS = ["file", "condition", "replicate", "fraction"] + +_FILE_SECTION = ["Fraction_Group", "Fraction", "Spectra_Filepath", "Label", "Sample"] +_SAMPLE_SECTION = ["Sample", "MSstats_Condition", "MSstats_BioReplicate"] + + +def default_design(files: List[str]) -> pd.DataFrame: + """A design for ``files`` with no conditions set, one biological replicate per file and no fractions. + + Conditions are left empty rather than guessed from file names. + """ + names = [Path(f).name for f in files] + return pd.DataFrame({ + "file": names, + "condition": [""] * len(names), + "replicate": list(range(1, len(names) + 1)), + "fraction": [1] * len(names), + }) + + +def _normalize(design: pd.DataFrame) -> pd.DataFrame: + """Coerce a design to COLUMNS with string conditions and positive integer replicates and fractions.""" + out = design.reindex(columns=COLUMNS).copy() + out["file"] = out["file"].fillna("").astype(str).map(lambda f: Path(f).name if f else f) + out["condition"] = out["condition"].fillna("").astype(str).str.strip() + for col in ("replicate", "fraction"): + out[col] = pd.to_numeric(out[col], errors="coerce").fillna(1).astype(int).clip(lower=1) + return out.reset_index(drop=True) + + +def align_design(design: pd.DataFrame, files: List[str]) -> pd.DataFrame: + """The design restricted to ``files``, in their order, with default rows for files it does not list. + + Files are matched by name, so a design written on another machine (other + directories) still applies. + """ + design = _normalize(design).drop_duplicates("file", keep="first").set_index("file") + defaults = default_design(files).set_index("file") + rows = [design.loc[n] if n in design.index else defaults.loc[n] for n in defaults.index] + if not rows: + return pd.DataFrame(columns=COLUMNS) + return _normalize(pd.DataFrame(rows).rename_axis("file").reset_index()) + + +def _is_sdrf(header: List[str]) -> bool: + lowered = [h.strip().lower() for h in header] + return "source name" in lowered or any(h.startswith("comment[data file") for h in lowered) + + +def _from_sdrf(df: pd.DataFrame) -> pd.DataFrame: + """Design rows from an SDRF table (one row per file and label).""" + cols = {c.strip().lower(): c for c in df.columns} + + def first(*names): + for name in names: + if name in cols: + return df[cols[name]] + return None + + files = first("comment[data file]") + if files is None: + raise ValueError("SDRF has no 'comment[data file]' column.") + factors = [cols[c] for c in cols if c.startswith("factor value[")] + if factors: + condition = df[factors].astype(str).agg(" | ".join, axis=1) + else: + condition = pd.Series([""] * len(df)) + replicate = first("characteristics[biological replicate]", "comment[biological replicate]") + fraction = first("comment[fraction identifier]") + design = pd.DataFrame({ + "file": files, + "condition": condition, + "replicate": replicate if replicate is not None else 1, + "fraction": fraction if fraction is not None else 1, + }) + # Multiplexed SDRFs list a file once per label; the design is per file. + return design.drop_duplicates("file", keep="first") + + +def _from_openms(text: str) -> pd.DataFrame: + """Design rows from an OpenMS design (one-table, or file and sample tables separated by a blank line).""" + blocks = [b for b in text.replace("\r\n", "\n").split("\n\n") if b.strip()] + files = pd.read_csv(io.StringIO(blocks[0]), sep="\t", dtype=str) + if "Spectra_Filepath" not in files.columns: + raise ValueError("Not an OpenMS design: no 'Spectra_Filepath' column.") + if len(blocks) > 1: + samples = pd.read_csv(io.StringIO(blocks[1]), sep="\t", dtype=str) + files = files.merge(samples, on="Sample", how="left", suffixes=("", "_sample")) + condition = files["MSstats_Condition"] if "MSstats_Condition" in files.columns else files.get("Sample", "") + replicate = files["MSstats_BioReplicate"] if "MSstats_BioReplicate" in files.columns else 1 + design = pd.DataFrame({ + "file": files["Spectra_Filepath"], + # to_openms_design writes an unset condition as NA, MSstats' missing value + "condition": pd.Series(condition).replace("NA", ""), + "replicate": replicate, + "fraction": files["Fraction"] if "Fraction" in files.columns else 1, + }) + return design.drop_duplicates("file", keep="first") + + +def read_design(source: Union[str, Path, bytes]) -> pd.DataFrame: + """Parse an OpenMS design (one- or two-table) or an SDRF file into a design table. + + Args: + source: a path, or the file's content as text or bytes. + + Raises: + ValueError: if the content is neither format. + """ + if isinstance(source, bytes): + text = source.decode("utf-8-sig") + elif isinstance(source, Path) or (isinstance(source, str) and "\n" not in source and Path(source).is_file()): + text = Path(source).read_text(encoding="utf-8-sig") + else: + text = source + header = text.lstrip().split("\n", 1)[0].rstrip("\r").split("\t") + if _is_sdrf(header): + design = _from_sdrf(pd.read_csv(io.StringIO(text), sep="\t", dtype=str)) + else: + design = _from_openms(text) + return _normalize(design) + + +def to_openms_design(design: pd.DataFrame, paths: Union[List[str], None] = None) -> str: + """The design as an OpenMS two-table ``design.tsv`` (label-free, so Label is 1). + + Each condition and biological replicate is one Sample. Each technical + replicate of a sample is its own Fraction_Group, whose files are its fractions. + + Args: + design: the design table. + paths: full paths to write as Spectra_Filepath, matched by file name; + the bare file name is written for files not among them. + """ + design = _normalize(design) + by_name = {Path(p).name: str(p) for p in (paths or [])} + design["condition"] = design["condition"].replace("", "NA") + design["technical"] = design.groupby(["condition", "replicate", "fraction"]).cumcount() + design["Sample"] = design.groupby(["condition", "replicate"], sort=False).ngroup() + 1 + design["Fraction_Group"] = design.groupby(["condition", "replicate", "technical"], sort=False).ngroup() + 1 + file_section = pd.DataFrame({ + "Fraction_Group": design["Fraction_Group"], + "Fraction": design["fraction"], + "Spectra_Filepath": [by_name.get(f, f) for f in design["file"]], + "Label": 1, + "Sample": design["Sample"], + }).sort_values(["Fraction_Group", "Fraction"], kind="stable") + sample_section = ( + design.drop_duplicates("Sample") + .rename(columns={"condition": "MSstats_Condition", "replicate": "MSstats_BioReplicate"}) + [_SAMPLE_SECTION] + .sort_values("Sample") + ) + return ( + file_section[_FILE_SECTION].to_csv(sep="\t", index=False, lineterminator="\n") + + "\n" + + sample_section.to_csv(sep="\t", index=False, lineterminator="\n") + ) + + +def check_design(design: pd.DataFrame) -> List[str]: + """Notices about a design that is valid but probably not what was meant. + + None of these stop a run: a design with them still writes a loadable file. + """ + design = _normalize(design) + notices = [] + if design.empty: + return notices + unset = design.loc[design["condition"] == "", "file"].tolist() + if unset: + notices.append(f"{len(unset)} file(s) have no condition: {', '.join(unset[:3])}{' …' if len(unset) > 3 else ''}.") + dup = design.loc[design["file"].duplicated(), "file"].unique().tolist() + if dup: + notices.append(f"Listed more than once: {', '.join(dup)}.") + conditions = design.loc[design["condition"] != "", "condition"].nunique() + if conditions == 1 and not unset: + notices.append("All files are in one condition, so there is nothing to compare.") + tech = design.groupby(["condition", "replicate", "fraction"]).size() + if (tech > 1).any(): + notices.append( + f"{int((tech > 1).sum())} condition/replicate/fraction combination(s) have more than one file; " + "they are treated as technical replicates." + ) + if design["fraction"].max() > 1: + counts = {} + for (cond, rep), grp in design.groupby(["condition", "replicate"]): + fractions = sorted(set(grp["fraction"])) + if fractions != list(range(1, fractions[-1] + 1)): + missing = sorted(set(range(1, fractions[-1] + 1)) - set(fractions)) + notices.append(f"{cond or 'No condition'} replicate {rep} is missing fraction(s) {', '.join(map(str, missing))}.") + counts[(cond, rep)] = len(fractions) + if len(set(counts.values())) > 1: + notices.append("Samples have different numbers of fractions.") + return notices + + +def design_path(workflow_dir: Union[str, Path]) -> Path: + """Where a workflow's experimental design is stored (an OpenMS design.tsv).""" + return Path(workflow_dir, DESIGN_FILE) + + +def save_design(workflow_dir: Union[str, Path], design: pd.DataFrame, paths: Union[List[str], None] = None) -> Path: + """Write the design to the workflow directory as an OpenMS design.tsv and return its path.""" + path = design_path(workflow_dir) + path.write_text(to_openms_design(design, paths)) + return path + + +def load_design(workflow_dir: Union[str, Path], files: Union[List[str], None] = None) -> pd.DataFrame: + """The workflow's stored design, aligned to ``files`` when given. + + Returns the default design for ``files`` (or an empty table) when none is stored. + """ + path = design_path(workflow_dir) + design = read_design(path) if path.is_file() else pd.DataFrame(columns=COLUMNS) + if files is None: + return _normalize(design) + return align_design(design, files) diff --git a/src/workflow/StreamlitUI.py b/src/workflow/StreamlitUI.py index 787150afa..bec561cd5 100644 --- a/src/workflow/StreamlitUI.py +++ b/src/workflow/StreamlitUI.py @@ -11,6 +11,7 @@ import importlib.util import time from io import BytesIO +import pandas as pd import zipfile from datetime import datetime @@ -22,6 +23,15 @@ tk_file_dialog, ) from src.workflow._log_status import classify_log_outcome +from src.workflow.ExperimentalDesign import ( + align_design, + check_design, + design_path, + load_design, + read_design, + save_design, + to_openms_design, +) from src.workflow.ParameterManager import ( InvalidParameterFileError, TransientParameterFileError, @@ -689,16 +699,23 @@ def format_files(input: Any) -> List[str]: # argument is still hashed into the widget's element id. Since this method feeds # the persisted parameter straight back in as that argument, the id changed on # every interaction, the following interaction arrived under the now-stale id and - # was silently dropped: selecting six mzML files kept only three. Seed the widget - # on first render only; from then on session state owns the value. - def seed(**kwargs: Any) -> dict: - return {} if key in st.session_state else kwargs + # was silently dropped: selecting six mzML files kept only three. Passing the + # argument on the first render only is not enough either: the id still changes + # once, between that render and the next, and the next rerun of the enclosing + # fragment resets the widget to empty -- a selection restored from params.json + # was wiped (and saved as empty) by the first reactive rerun of configure(). + # So the argument is never passed: the first render writes the value into + # session state, which then owns it, and the id never changes. + def seed(initial: Any) -> dict: + if key not in st.session_state: + st.session_state[key] = initial + return {} if widget_type == "text": - st.text_input(name, key=key, help=help, on_change=on_change, **seed(value=value)) + st.text_input(name, key=key, help=help, on_change=on_change, **seed(value)) elif widget_type == "textarea": - st.text_area(name, key=key, help=help, on_change=on_change, **seed(value=value)) + st.text_area(name, key=key, help=help, on_change=on_change, **seed(value)) elif widget_type == "number": number_type = float if isinstance(value, float) else int @@ -719,11 +736,11 @@ def seed(**kwargs: Any) -> dict: key=key, help=help, on_change=on_change, - **seed(value=value), + **seed(value), ) elif widget_type == "checkbox": - st.checkbox(name, key=key, help=help, on_change=on_change, **seed(value=value)) + st.checkbox(name, key=key, help=help, on_change=on_change, **seed(value)) elif widget_type == "selectbox": if options is not None: @@ -734,7 +751,7 @@ def seed(**kwargs: Any) -> dict: format_func=format_files, help=help, on_change=on_change, - **seed(index=options.index(value) if value in options else 0), + **seed(value if value in options else (options[0] if options else None)), ) else: st.warning(f"Select widget '{name}' requires options parameter") @@ -748,7 +765,7 @@ def seed(**kwargs: Any) -> dict: format_func=format_files, help=help, on_change=on_change, - **seed(default=value), + **seed(value), ) else: st.warning(f"Select widget '{name}' requires options parameter") @@ -770,7 +787,7 @@ def seed(**kwargs: Any) -> dict: format=None, help=help, on_change=on_change, - **seed(value=value), + **seed(value), ) else: st.warning( @@ -778,12 +795,12 @@ def seed(**kwargs: Any) -> dict: ) elif widget_type == "password": - st.text_input(name, type="password", key=key, help=help, on_change=on_change, **seed(value=value)) + st.text_input(name, type="password", key=key, help=help, on_change=on_change, **seed(value)) elif widget_type == "auto": # Auto-determine widget type based on value if isinstance(value, bool): - st.checkbox(name, key=key, help=help, on_change=on_change, **seed(value=value)) + st.checkbox(name, key=key, help=help, on_change=on_change, **seed(value)) elif isinstance(value, (int, float)): self._input_widget_impl( key, @@ -1419,6 +1436,94 @@ def input_python( cols = st.columns(num_cols) self.parameter_manager.save_parameters() + def experimental_design(self, key: str, name: str = "Experimental design") -> None: + """ + Table assigning each selected input file a condition, biological replicate and fraction. + + Edits are saved to ``/experimental-design.tsv`` in the + OpenMS design format, so ``execution()`` can pass that file to a TOPP + tool's ``-design`` or read it with ``ExperimentalDesign.load_design()``. + A design.tsv (one- or two-table) or SDRF file can be imported, and + spreadsheet cells can be pasted into the table. Inconsistencies such as + missing fractions are shown as notices, not errors. + + Args: + key (str): The input file key whose selection the design covers + (the key passed to ``select_input_file``). Pass ``reactive=True`` + to that ``select_input_file`` so the table follows the selection. + name (str, optional): The heading shown above the table. + """ + files = st.session_state.get( + f"{self.parameter_manager.param_prefix}{key}", self.params.get(key, []) + ) + if isinstance(files, str): + files = [files] + st.markdown(f"**{name}**") + if not files: + st.info("Select input files to define the experimental design.") + return + + # Widget state keys stay outside param_prefix: save_parameters() writes + # every session key with that prefix to params.json. + version_key = f"_design-{Path(self.workflow_dir).name}-{key}" + st.session_state.setdefault(version_key, 0) + stored = load_design(self.workflow_dir, files) + + uploaded = st.file_uploader( + "Import design.tsv or SDRF", + type=["tsv", "txt", "sdrf"], + key=f"{version_key}-import", + help="An OpenMS experimental design (one- or two-table) or an SDRF file. " + "Files are matched by name; files it does not list keep their current row.", + ) + applied_key = f"{version_key}-applied" + if uploaded is not None and st.session_state.get(applied_key) != uploaded.file_id: + st.session_state[applied_key] = uploaded.file_id + try: + imported = read_design(uploaded.getvalue()) + except (ValueError, KeyError, pd.errors.ParserError) as e: + st.error(f"Could not read {uploaded.name}: {e}") + else: + listed = set(imported["file"]) + kept = stored[~stored["file"].isin(listed)] + stored = align_design(pd.concat([imported, kept]), files) + matched = len(listed & set(stored["file"])) + save_design(self.workflow_dir, stored, files) + st.session_state[version_key] += 1 + st.toast(f"Imported the design of {matched} of {len(files)} file(s)") + + edited = st.data_editor( + stored, + key=f"{version_key}-{st.session_state[version_key]}", + hide_index=True, + num_rows="fixed", + disabled=["file"], + column_config={ + "file": st.column_config.TextColumn("File"), + "condition": st.column_config.TextColumn( + "Condition", help="The group this file is compared as, e.g. control or treated." + ), + "replicate": st.column_config.NumberColumn( + "Replicate", help="Biological replicate within the condition.", min_value=1, step=1 + ), + "fraction": st.column_config.NumberColumn( + "Fraction", help="Fraction number; 1 for unfractionated samples.", min_value=1, step=1 + ), + }, + ) + path = design_path(self.workflow_dir) + if not path.is_file() or path.read_text() != to_openms_design(edited, files): + save_design(self.workflow_dir, edited, files) + + for notice in check_design(edited): + st.info(notice, icon="ℹ️") + st.download_button( + "Download design.tsv", + to_openms_design(edited, files), + file_name="design.tsv", + mime="text/tab-separated-values", + ) + def zip_and_download_files(self, directory: str): """ Creates a zip archive of all files within a specified directory, diff --git a/tests/test_experimental_design.py b/tests/test_experimental_design.py new file mode 100644 index 000000000..43754e09a --- /dev/null +++ b/tests/test_experimental_design.py @@ -0,0 +1,110 @@ +"""Tests for src/workflow/ExperimentalDesign.py (Streamlit-free).""" +import pandas as pd +import pyopenms as poms + +from src.workflow.ExperimentalDesign import ( + COLUMNS, + align_design, + check_design, + default_design, + load_design, + read_design, + save_design, + to_openms_design, +) + +FILES = ["/data/ctrl_1.mzML", "/data/ctrl_2.mzML", "/data/treat_1.mzML", "/data/treat_2.mzML"] + + +def _design(): + return pd.DataFrame({ + "file": ["ctrl_1.mzML", "ctrl_2.mzML", "treat_1.mzML", "treat_2.mzML"], + "condition": ["ctrl", "ctrl", "treat", "treat"], + "replicate": [1, 2, 1, 2], + "fraction": [1, 1, 1, 1], + }) + + +def test_default_design_does_not_guess_conditions(): + design = default_design(FILES) + assert list(design.columns) == COLUMNS + assert list(design["file"]) == ["ctrl_1.mzML", "ctrl_2.mzML", "treat_1.mzML", "treat_2.mzML"] + assert set(design["condition"]) == {""} + assert list(design["replicate"]) == [1, 2, 3, 4] + + +def test_openms_design_round_trip_and_loads_in_pyopenms(tmp_path): + text = to_openms_design(_design(), FILES) + path = tmp_path / "design.tsv" + path.write_text(text) + ed = poms.ExperimentalDesignFile.load(str(path), False) + assert ed.getNumberOfMSFiles() == 4 + assert ed.getNumberOfSamples() == 4 + assert "/data/ctrl_1.mzML" in text + pd.testing.assert_frame_equal(read_design(path), _design()) + + +def test_fractions_and_technical_replicates_map_to_fraction_groups(tmp_path): + design = pd.DataFrame({ + "file": ["a_f1.mzML", "a_f2.mzML", "a_f1_tech.mzML", "a_f2_tech.mzML"], + "condition": ["A"] * 4, + "replicate": [1, 1, 1, 1], + "fraction": [1, 2, 1, 2], + }) + path = tmp_path / "design.tsv" + path.write_text(to_openms_design(design)) + ed = poms.ExperimentalDesignFile.load(str(path), False) + assert ed.isFractionated() + assert ed.getNumberOfFractionGroups() == 2 # two technical replicates + assert ed.getNumberOfSamples() == 1 + + +def test_read_one_table_openms_design(): + text = ( + "Fraction_Group\tFraction\tSpectra_Filepath\tLabel\tSample\tMSstats_Condition\tMSstats_BioReplicate\n" + "1\t1\t/x/ctrl_1.mzML\t1\t1\tctrl\t1\n" + "2\t1\t/x/treat_1.mzML\t1\t2\ttreat\t1\n" + ) + design = read_design(text) + assert list(design["file"]) == ["ctrl_1.mzML", "treat_1.mzML"] + assert list(design["condition"]) == ["ctrl", "treat"] + + +def test_read_sdrf_uses_factor_values_and_dedups_labels(): + text = ( + "source name\tcharacteristics[biological replicate]\tcomment[label]\tcomment[data file]" + "\tcomment[fraction identifier]\tfactor value[treatment]\n" + "s1\t1\tlabel free sample\tctrl_1.raw\t1\tcontrol\n" + "s2\t2\tlabel free sample\tctrl_2.raw\t2\tcontrol\n" + "s2\t2\tlabel free sample\tctrl_2.raw\t2\tcontrol\n" + ) + design = read_design(text.encode()) + assert list(design["file"]) == ["ctrl_1.raw", "ctrl_2.raw"] + assert list(design["condition"]) == ["control", "control"] + assert list(design["replicate"]) == [1, 2] + assert list(design["fraction"]) == [1, 2] + + +def test_align_design_matches_by_name_and_fills_defaults(): + imported = _design().iloc[:2].assign(file=lambda d: "/elsewhere/" + d["file"]) + aligned = align_design(imported, FILES[:3]) + assert list(aligned["file"]) == ["ctrl_1.mzML", "ctrl_2.mzML", "treat_1.mzML"] + assert list(aligned["condition"]) == ["ctrl", "ctrl", ""] + + +def test_check_design_notices(): + assert check_design(_design()) == [] + notices = check_design(default_design(FILES)) + assert any("no condition" in n for n in notices) + gaps = _design().assign(fraction=[1, 3, 1, 1], replicate=[1, 1, 1, 2]) + notices = check_design(gaps) + assert any("missing fraction(s) 2" in n for n in notices) + assert any("different numbers of fractions" in n for n in notices) + + +def test_save_and_load_design(tmp_path): + save_design(tmp_path, _design(), FILES) + loaded = load_design(tmp_path, FILES[::-1]) + assert list(loaded["file"]) == ["treat_2.mzML", "treat_1.mzML", "ctrl_2.mzML", "ctrl_1.mzML"] + assert list(loaded["condition"]) == ["treat", "treat", "ctrl", "ctrl"] + assert load_design(tmp_path / "missing", FILES)["condition"].eq("").all() diff --git a/tests/test_input_widget_seed.py b/tests/test_input_widget_seed.py new file mode 100644 index 000000000..7e8a6c40d --- /dev/null +++ b/tests/test_input_widget_seed.py @@ -0,0 +1,45 @@ +"""A value restored from params.json must survive reruns of input_widget. + +On Streamlit < 1.50 a widget's initial-value argument is hashed into its +element id. Passing it on the first render only still changed the id once, +and the following rerun reset the widget: a restored multiselect came back +empty and was saved as empty. +""" +from streamlit.testing.v1 import AppTest + + +def _script(): + from types import SimpleNamespace + + from src.workflow.StreamlitUI import StreamlitUI + + ui = object.__new__(StreamlitUI) + ui.params = {"files": ["a", "b"], "mode": "y", "n": 3} + ui.parameter_manager = SimpleNamespace(param_prefix="p-", save_parameters=lambda: None) + for key, widget_type, options in ( + ("files", "multiselect", ["a", "b", "c"]), + ("mode", "selectbox", ["x", "y"]), + ("n", "number", None), + ): + ui._input_widget_impl( + key, None, key, None, widget_type, options, None, None, 1, True, None + ) + + +def test_restored_values_survive_reruns(): + at = AppTest.from_function(_script) + at.run() + for _ in range(2): + assert at.multiselect(key="p-files").value == ["a", "b"] + assert at.selectbox(key="p-mode").value == "y" + assert at.number_input(key="p-n").value == 3 + at.run() + assert not at.exception + + +def test_user_change_is_kept(): + at = AppTest.from_function(_script) + at.run() + at.multiselect(key="p-files").unselect("a").run() + at.run() + assert at.multiselect(key="p-files").value == ["b"] From 75835d40c5d2b59b98cc380e4dde842b8bd81b39 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Oct 2026 06:23:36 +0000 Subject: [PATCH 2/2] Experimental design: Windows paths, duplicate file names, UTF-8, empty files - Split file names on both / and \ so a design.tsv written on Windows matches on a Linux host. - align_design merges instead of indexing by name, so two selected files with the same name no longer crash it, and to_openms_design keeps each row's own path when given the files in row order. - Read and write experimental-design.tsv as UTF-8. - An empty upload raises ValueError and shows "Could not read" instead of an IndexError. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01D5Wo2FHPpi5a8X6GppNxEk --- src/workflow/ExperimentalDesign.py | 41 ++++++++++++++++++++---------- src/workflow/StreamlitUI.py | 2 +- tests/test_experimental_design.py | 31 ++++++++++++++++++++++ 3 files changed, 60 insertions(+), 14 deletions(-) diff --git a/src/workflow/ExperimentalDesign.py b/src/workflow/ExperimentalDesign.py index aad1982bc..fcca8878f 100644 --- a/src/workflow/ExperimentalDesign.py +++ b/src/workflow/ExperimentalDesign.py @@ -12,6 +12,7 @@ worker. """ import io +import re from pathlib import Path from typing import List, Union @@ -24,12 +25,17 @@ _SAMPLE_SECTION = ["Sample", "MSstats_Condition", "MSstats_BioReplicate"] +def _file_name(path: str) -> str: + """The last component of a path, splitting on / and \\ so Windows paths work on any host.""" + return re.split(r"[\\/]", path)[-1] + + def default_design(files: List[str]) -> pd.DataFrame: """A design for ``files`` with no conditions set, one biological replicate per file and no fractions. Conditions are left empty rather than guessed from file names. """ - names = [Path(f).name for f in files] + names = [_file_name(str(f)) for f in files] return pd.DataFrame({ "file": names, "condition": [""] * len(names), @@ -41,7 +47,7 @@ def default_design(files: List[str]) -> pd.DataFrame: def _normalize(design: pd.DataFrame) -> pd.DataFrame: """Coerce a design to COLUMNS with string conditions and positive integer replicates and fractions.""" out = design.reindex(columns=COLUMNS).copy() - out["file"] = out["file"].fillna("").astype(str).map(lambda f: Path(f).name if f else f) + out["file"] = out["file"].fillna("").astype(str).map(lambda f: _file_name(f) if f else f) out["condition"] = out["condition"].fillna("").astype(str).str.strip() for col in ("replicate", "fraction"): out[col] = pd.to_numeric(out[col], errors="coerce").fillna(1).astype(int).clip(lower=1) @@ -54,12 +60,11 @@ def align_design(design: pd.DataFrame, files: List[str]) -> pd.DataFrame: Files are matched by name, so a design written on another machine (other directories) still applies. """ - design = _normalize(design).drop_duplicates("file", keep="first").set_index("file") - defaults = default_design(files).set_index("file") - rows = [design.loc[n] if n in design.index else defaults.loc[n] for n in defaults.index] - if not rows: - return pd.DataFrame(columns=COLUMNS) - return _normalize(pd.DataFrame(rows).rename_axis("file").reset_index()) + design = _normalize(design).drop_duplicates("file", keep="first") + out = default_design(files).merge(design, on="file", how="left", suffixes=("_default", "")) + for col in COLUMNS[1:]: + out[col] = out[col].where(out[col].notna(), out[f"{col}_default"]) + return _normalize(out) def _is_sdrf(header: List[str]) -> bool: @@ -100,6 +105,8 @@ def first(*names): def _from_openms(text: str) -> pd.DataFrame: """Design rows from an OpenMS design (one-table, or file and sample tables separated by a blank line).""" blocks = [b for b in text.replace("\r\n", "\n").split("\n\n") if b.strip()] + if not blocks: + raise ValueError("The file is empty.") files = pd.read_csv(io.StringIO(blocks[0]), sep="\t", dtype=str) if "Spectra_Filepath" not in files.columns: raise ValueError("Not an OpenMS design: no 'Spectra_Filepath' column.") @@ -149,11 +156,19 @@ def to_openms_design(design: pd.DataFrame, paths: Union[List[str], None] = None) Args: design: the design table. - paths: full paths to write as Spectra_Filepath, matched by file name; - the bare file name is written for files not among them. + paths: full paths to write as Spectra_Filepath. When they are the + design's files in row order (as for the selected input files), + each row keeps its own path, even if two share a file name; + otherwise they are matched by file name, and the bare file name + is written for files not among them. """ design = _normalize(design) - by_name = {Path(p).name: str(p) for p in (paths or [])} + paths = [str(p) for p in (paths or [])] + if [_file_name(p) for p in paths] == list(design["file"]): + spectra = paths + else: + by_name = {_file_name(p): p for p in paths} + spectra = [by_name.get(f, f) for f in design["file"]] design["condition"] = design["condition"].replace("", "NA") design["technical"] = design.groupby(["condition", "replicate", "fraction"]).cumcount() design["Sample"] = design.groupby(["condition", "replicate"], sort=False).ngroup() + 1 @@ -161,7 +176,7 @@ def to_openms_design(design: pd.DataFrame, paths: Union[List[str], None] = None) file_section = pd.DataFrame({ "Fraction_Group": design["Fraction_Group"], "Fraction": design["fraction"], - "Spectra_Filepath": [by_name.get(f, f) for f in design["file"]], + "Spectra_Filepath": spectra, "Label": 1, "Sample": design["Sample"], }).sort_values(["Fraction_Group", "Fraction"], kind="stable") @@ -223,7 +238,7 @@ def design_path(workflow_dir: Union[str, Path]) -> Path: def save_design(workflow_dir: Union[str, Path], design: pd.DataFrame, paths: Union[List[str], None] = None) -> Path: """Write the design to the workflow directory as an OpenMS design.tsv and return its path.""" path = design_path(workflow_dir) - path.write_text(to_openms_design(design, paths)) + path.write_text(to_openms_design(design, paths), encoding="utf-8") return path diff --git a/src/workflow/StreamlitUI.py b/src/workflow/StreamlitUI.py index bec561cd5..14f74a802 100644 --- a/src/workflow/StreamlitUI.py +++ b/src/workflow/StreamlitUI.py @@ -1512,7 +1512,7 @@ def experimental_design(self, key: str, name: str = "Experimental design") -> No }, ) path = design_path(self.workflow_dir) - if not path.is_file() or path.read_text() != to_openms_design(edited, files): + if not path.is_file() or path.read_text(encoding="utf-8") != to_openms_design(edited, files): save_design(self.workflow_dir, edited, files) for notice in check_design(edited): diff --git a/tests/test_experimental_design.py b/tests/test_experimental_design.py index 43754e09a..20172a498 100644 --- a/tests/test_experimental_design.py +++ b/tests/test_experimental_design.py @@ -108,3 +108,34 @@ def test_save_and_load_design(tmp_path): assert list(loaded["file"]) == ["treat_2.mzML", "treat_1.mzML", "ctrl_2.mzML", "ctrl_1.mzML"] assert list(loaded["condition"]) == ["treat", "treat", "ctrl", "ctrl"] assert load_design(tmp_path / "missing", FILES)["condition"].eq("").all() + + +def test_windows_paths_match_by_file_name(): + text = ( + "Fraction_Group\tFraction\tSpectra_Filepath\tLabel\tSample\tMSstats_Condition\tMSstats_BioReplicate\n" + "1\t1\tC:\\data\\ctrl_1.mzML\t1\t1\tctrl\t1\n" + ) + assert list(align_design(read_design(text), FILES[:1])["condition"]) == ["ctrl"] + + +def test_empty_file_raises_value_error(): + for content in (b"", b" \n\n "): + try: + read_design(content) + except ValueError: + continue + raise AssertionError("expected ValueError") + + +def test_duplicate_file_names_keep_their_own_paths(): + files = ["/run1/a.mzML", "/run2/a.mzML", "/run1/b.mzML"] + design = align_design(pd.DataFrame(columns=COLUMNS), files) + assert list(design["file"]) == ["a.mzML", "a.mzML", "b.mzML"] + text = to_openms_design(design, files) + assert "/run1/a.mzML" in text and "/run2/a.mzML" in text + assert any("more than once" in n for n in check_design(design)) + + +def test_non_ascii_conditions_round_trip(tmp_path): + save_design(tmp_path, _design().assign(condition=["5 µM", "5 µM", "10 µM", "10 µM"]), FILES) + assert list(load_design(tmp_path, FILES)["condition"]) == ["5 µM", "5 µM", "10 µM", "10 µM"]