From 143c556d43b9bf38390b1728218b1e592c07a644 Mon Sep 17 00:00:00 2001 From: Mohamed Sameh Date: Thu, 3 Sep 2026 00:13:03 +0200 Subject: [PATCH 1/5] feat: accept rt_norm_pred as a retention time column --- alphabase/spectral_library/translate_core.py | 14 ++++++--- tests/unit/spectral_library/test_translate.py | 31 ++++++++++++------- .../spectral_library/test_translate_diann.py | 30 +++++++++++++----- 3 files changed, 51 insertions(+), 24 deletions(-) diff --git a/alphabase/spectral_library/translate_core.py b/alphabase/spectral_library/translate_core.py index 360c9369..5f47abbd 100644 --- a/alphabase/spectral_library/translate_core.py +++ b/alphabase/spectral_library/translate_core.py @@ -21,11 +21,15 @@ from alphabase.psm_reader.keys import ConstantsClass, PsmDfCols from alphabase.utils import explode_multiple_columns -# Candidate precursor columns in order of precedence, for libraries that carry more than -# one. The `*_pred` names are peptdeep's prediction outputs, which take priority over a -# measured value; `irt_pred` outranks `rt_pred` because an indexed RT is what a -# third-party library wants. -RT_COLUMNS = ["irt_pred", "rt_pred", PsmDfCols.RT, "irt", PsmDfCols.RT_NORM] +# Candidate precursor columns, in order of precedence. +RT_COLUMNS = [ + "irt_pred", + "rt_pred", + "rt_norm_pred", + PsmDfCols.RT, + "irt", + PsmDfCols.RT_NORM, +] MOBILITY_COLUMNS = ["mobility_pred", PsmDfCols.MOBILITY] CCS_COLUMNS = ["ccs_pred", PsmDfCols.CCS] diff --git a/tests/unit/spectral_library/test_translate.py b/tests/unit/spectral_library/test_translate.py index 3ac089ce..8f396448 100644 --- a/tests/unit/spectral_library/test_translate.py +++ b/tests/unit/spectral_library/test_translate.py @@ -3,11 +3,9 @@ These tests pin the behaviour of `alphabase.spectral_library.translate` as it is today, so that the upcoming restructuring can be shown to change nothing. -Two tests still pin *buggy* behaviour that a later commit fixes. Each carries a -`CHARACTERIZATION (bug)` note in its docstring: - -1. `rt_norm_pred` is not accepted as a retention time column -2. the exploded fragment columns are object dtype, with a string `FragmentCharge` +One test still pins *buggy* behaviour that a later commit fixes, marked +`CHARACTERIZATION (bug)` in its docstring: the exploded fragment columns are +object dtype, with a string `FragmentCharge`. One more, `test_speclib_to_swath_df_returns_none`, pins a function that a later commit removes rather than fixes. @@ -169,15 +167,16 @@ def test_modified_sequence_uses_translate_mod_dict() -> None: @pytest.mark.parametrize( ("present", "expected"), [ - (["irt_pred", "rt_pred", "rt", "irt", "rt_norm"], "irt_pred"), - (["rt_pred", "rt", "irt", "rt_norm"], "rt_pred"), + (["irt_pred", "rt_pred", "rt_norm_pred", "rt", "irt", "rt_norm"], "irt_pred"), + (["rt_pred", "rt_norm_pred", "rt", "irt", "rt_norm"], "rt_pred"), + (["rt_norm_pred", "rt", "irt", "rt_norm"], "rt_norm_pred"), (["rt", "irt", "rt_norm"], "rt"), (["irt", "rt_norm"], "irt"), (["rt_norm"], "rt_norm"), ], ) def test_rt_column_precedence(present: list, expected: str) -> None: - """RT is taken from the first present of irt_pred, rt_pred, rt, irt, rt_norm.""" + """RT comes from the first present candidate, predictions before measurements.""" speclib = _build_speclib() # give each candidate column a distinct value, so `RT` identifies its source values = {name: float(i + 1) for i, name in enumerate(present)} @@ -188,17 +187,25 @@ def test_rt_column_precedence(present: list, expected: str) -> None: assert _export(speclib)["RT"].unique().tolist() == [values[expected]] -def test_rt_norm_pred_is_not_accepted() -> None: - """CHARACTERIZATION (bug): `rt_norm_pred` is not a recognised RT column. +def test_rt_norm_pred_is_accepted() -> None: + """A library carrying only `rt_norm_pred` exports, rather than being rejected. - peptdeep writes it alongside `rt_pred`, so a library carrying only - `rt_norm_pred` is rejected. A later commit adds it to the candidates. + peptdeep writes `rt_norm_pred` alongside `rt_pred`, so this matters for a + library whose `rt_pred` was dropped. """ speclib = _build_speclib() speclib._precursor_df = speclib._precursor_df.rename( columns={"rt_pred": "rt_norm_pred"} ) + assert _export(speclib)["RT"].notna().all() + + +def test_export_without_any_rt_column_is_rejected() -> None: + """A library with no retention time at all is still an error.""" + speclib = _build_speclib() + speclib._precursor_df = speclib._precursor_df.drop(columns=["rt_pred"]) + with pytest.raises(ValueError, match="must contain the RT columns"): _export(speclib) diff --git a/tests/unit/spectral_library/test_translate_diann.py b/tests/unit/spectral_library/test_translate_diann.py index 18b9e35c..b79cc3eb 100644 --- a/tests/unit/spectral_library/test_translate_diann.py +++ b/tests/unit/spectral_library/test_translate_diann.py @@ -150,15 +150,16 @@ def _keys(df: pd.DataFrame) -> set: @pytest.mark.parametrize( ("present", "expected"), [ - (["irt_pred", "rt_pred", "rt", "irt", "rt_norm"], "irt_pred"), - (["rt_pred", "rt", "irt", "rt_norm"], "rt_pred"), + (["irt_pred", "rt_pred", "rt_norm_pred", "rt", "irt", "rt_norm"], "irt_pred"), + (["rt_pred", "rt_norm_pred", "rt", "irt", "rt_norm"], "rt_pred"), + (["rt_norm_pred", "rt", "irt", "rt_norm"], "rt_norm_pred"), (["rt", "irt", "rt_norm"], "rt"), (["irt", "rt_norm"], "irt"), (["rt_norm"], "rt_norm"), ], ) def test_speclib_to_diann_df_rt_column_precedence(present: list, expected: str) -> None: - """RT is taken from the first present of irt_pred, rt_pred, rt, irt, rt_norm.""" + """RT comes from the first present candidate, predictions before measurements.""" speclib = _build_speclib() # give each candidate column a distinct value, so `RT` identifies its source values = {name: float(i + 1) for i, name in enumerate(present)} @@ -174,15 +175,30 @@ def test_speclib_to_diann_df_rt_column_precedence(present: list, expected: str) assert df["RT"].unique().tolist() == [values[expected]] -def test_speclib_to_diann_df_rejects_rt_norm_pred() -> None: - """CHARACTERIZATION (bug): `rt_norm_pred` is not a recognised RT column. +def test_speclib_to_diann_df_accepts_rt_norm_pred() -> None: + """A library carrying only `rt_norm_pred` exports, rather than being rejected. - peptdeep writes it alongside `rt_pred`, so a library carrying only - `rt_norm_pred` is rejected. A later commit adds it to the candidates. + peptdeep writes `rt_norm_pred` alongside `rt_pred`, so this matters for a + library whose `rt_pred` was dropped. """ speclib = _build_speclib() speclib._precursor_df = speclib._precursor_df.rename(columns={"rt": "rt_norm_pred"}) + df = speclib_to_diann_df( + speclib, + min_frag_mz=0, + max_frag_mz=np.inf, + min_frag_intensity=0.0, + verbose=False, + ) + assert df["RT"].notna().all() + + +def test_speclib_to_diann_df_without_any_rt_column_is_rejected() -> None: + """A library with no retention time at all is still an error.""" + speclib = _build_speclib() + speclib._precursor_df = speclib._precursor_df.drop(columns=["rt"]) + with pytest.raises(ValueError, match="must contain a retention time column"): speclib_to_diann_df( speclib, From 21f5412a6e86860fb519ac577e7e6e8e56bdb272 Mon Sep 17 00:00:00 2001 From: Mohamed Sameh Date: Thu, 3 Sep 2026 00:20:58 +0200 Subject: [PATCH 2/5] fix: flag one DIA-NN base peak per precursor row `Flags` marked the base peak by grouping on `Precursor.Id`, which is not unique: `SpecLibBase.append` produces libraries holding the same precursor twice, and those shared a single flag between them. --- alphabase/spectral_library/translate_diann.py | 8 +++++--- .../spectral_library/test_translate_diann.py | 20 +++++++++++-------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/alphabase/spectral_library/translate_diann.py b/alphabase/spectral_library/translate_diann.py index c2b5d34c..bdaf5ffb 100644 --- a/alphabase/spectral_library/translate_diann.py +++ b/alphabase/spectral_library/translate_diann.py @@ -62,8 +62,10 @@ class DiannParquetCols(metaclass=ConstantsClass): SOURCE_ID = "Source.Id" -# the DIA-NN names for the canonical fragment columns, in output order +# the DIA-NN names for the canonical fragment columns, in output order. `precursor_row` +# keeps its name for the base-peak flag; the schema selection at the end drops it. DIANN_FRAGMENT_COLUMNS = { + FragmentTableCols.PRECURSOR_ROW: FragmentTableCols.PRECURSOR_ROW, FragmentTableCols.FRAG_TYPE: DiannParquetCols.FRAGMENT_TYPE, FragmentTableCols.MZ: DiannParquetCols.PRODUCT_MZ, FragmentTableCols.INTENSITY: DiannParquetCols.RELATIVE_INTENSITY, @@ -227,10 +229,10 @@ def _precursors_to_diann_df( # noqa: PLR0913 ] = modloss df = df.reset_index(drop=True) - # Flags: base bit on all fragments, base-peak bit on each precursor's most intense one df[DiannParquetCols.FLAGS] = _DIANN_FLAG_BASE if len(df): - base_peak_idx = df.groupby(DiannParquetCols.PRECURSOR_ID, sort=False)[ + # by row: `Precursor.Id` repeats when a library holds the same precursor twice + base_peak_idx = df.groupby(FragmentTableCols.PRECURSOR_ROW, sort=False)[ DiannParquetCols.RELATIVE_INTENSITY ].idxmax() df.loc[base_peak_idx, DiannParquetCols.FLAGS] |= _DIANN_FLAG_FIRST_FRAGMENT diff --git a/tests/unit/spectral_library/test_translate_diann.py b/tests/unit/spectral_library/test_translate_diann.py index b79cc3eb..62410265 100644 --- a/tests/unit/spectral_library/test_translate_diann.py +++ b/tests/unit/spectral_library/test_translate_diann.py @@ -209,13 +209,12 @@ def test_speclib_to_diann_df_without_any_rt_column_is_rejected() -> None: ) -def test_speclib_to_diann_df_flags_group_by_precursor_id() -> None: - """CHARACTERIZATION (bug): duplicate precursors share one base-peak flag. +def test_speclib_to_diann_df_flags_one_base_peak_per_precursor_row() -> None: + """Duplicate precursors each get their own base-peak flag. - `Flags` marks the base peak by grouping on `Precursor.Id`, so a library - holding the same precursor twice -- which `SpecLibBase.append` produces -- - gets one flag for the pair instead of one per precursor row. A later commit - groups by precursor row instead. + `Flags` marks the base peak per precursor *row*, so a library holding the same + precursor twice -- which `SpecLibBase.append` produces -- gets one flag each + rather than one for the pair. `Precursor.Id` cannot distinguish them. """ speclib = _build_speclib() speclib.append(_build_speclib()) @@ -229,9 +228,14 @@ def test_speclib_to_diann_df_flags_group_by_precursor_id() -> None: verbose=False, ) - # every precursor row is exported, but only the distinct ids get a base peak + # the duplicated precursors share an id, so the id count is half the row count assert df["Precursor.Id"].nunique() == n_precursors // 2 - assert int((df["Flags"] & (1 << 4) > 0).sum()) == n_precursors // 2 + assert int((df["Flags"] & (1 << 4) > 0).sum()) == n_precursors + + # and each flagged fragment is the most intense of its own precursor's block + flagged = df["Flags"] & (1 << 4) > 0 + for _, group in df.groupby("Precursor.Id", sort=False): + assert group[flagged.loc[group.index]].shape[0] == 2 def test_speclib_to_diann_df_leaves_the_source_library_untouched() -> None: From 617d0614e2ea0e202199869b51201e746e691fbd Mon Sep 17 00:00:00 2001 From: Mohamed Sameh Date: Thu, 3 Sep 2026 00:44:18 +0200 Subject: [PATCH 3/5] fix: type the exported fragment columns, and notice a dead tsv writer `fragment_table` built a frame of lists and exploded it, and `explode` yields object columns. The DIA-NN export casts them against its schema; the SWATH one does not, so it returned `FragmentMz`, `RelativeIntensity`, `FragmentCharge` and `FragmentNumber` as objects of boxed scalars -- and the charge as strings, since it is sliced out of a fragment column name and was never converted. Concatenate the columns instead of exploding them: m/z and intensity keep the dtype of the frame they came from, the charge and series number become int64. That also removes the all-NaN row the explode needed dropped. `translate_to_tsv` never checked its writer process. `multiprocessing=True` is the default, and that process dies before writing anything if the calling script has no `if __name__ == "__main__":` guard, so the export printed its success message and returned over a 0-byte tsv. Raise on a non-zero exit code. The written tsv and parquet are byte-identical; only the in-memory frames change, and only in dtype -- plus `FragmentCharge` "1" -> 1. Co-Authored-By: Claude Opus 5 (1M context) --- alphabase/spectral_library/translate.py | 8 ++ alphabase/spectral_library/translate_core.py | 58 ++++++-------- tests/unit/spectral_library/test_translate.py | 80 ++++++++++++------- 3 files changed, 83 insertions(+), 63 deletions(-) diff --git a/alphabase/spectral_library/translate.py b/alphabase/spectral_library/translate.py index 57d693a0..3b281a57 100644 --- a/alphabase/spectral_library/translate.py +++ b/alphabase/spectral_library/translate.py @@ -295,3 +295,11 @@ def translate_to_tsv( "Translation finished, it will take several minutes to export the rest precursors to the tsv file..." ) writing_process.join() + if writing_process.exitcode: + raise RuntimeError( + f"the process writing {tsv} exited with code " + f"{writing_process.exitcode}, so the file is incomplete; its traceback " + 'is above. A script with no `if __name__ == "__main__":` guard is the ' + "usual cause on macOS and Windows. Pass multiprocessing=False to write " + "from this process instead." + ) diff --git a/alphabase/spectral_library/translate_core.py b/alphabase/spectral_library/translate_core.py index 5f47abbd..642899ac 100644 --- a/alphabase/spectral_library/translate_core.py +++ b/alphabase/spectral_library/translate_core.py @@ -19,7 +19,6 @@ from alphabase.numba_wrapper import numba_njit from alphabase.peptide.precursor import update_precursor_mz from alphabase.psm_reader.keys import ConstantsClass, PsmDfCols -from alphabase.utils import explode_multiple_columns # Candidate precursor columns, in order of precedence. RT_COLUMNS = [ @@ -60,17 +59,6 @@ class FragmentTableCols(metaclass=ConstantsClass): LOSS_TYPE = "loss_type" -# the per-fragment columns, in the order the exports emit them -FRAGMENT_VALUE_COLUMNS = [ - FragmentTableCols.FRAG_TYPE, - FragmentTableCols.MZ, - FragmentTableCols.INTENSITY, - FragmentTableCols.CHARGE, - FragmentTableCols.SERIES_NUMBER, - FragmentTableCols.LOSS_TYPE, -] - - def get_precursor_mz(precursor_df: pd.DataFrame) -> pd.Series: """Return the precursors' m/z, leaving `precursor_df` alone. @@ -190,7 +178,8 @@ def _get_frag_info_from_column_name(column: str) -> tuple: """Split a fragment column name into `(frag_type, loss_type, charge)`. For example `y_modloss_z2` -> `('y', 'modloss', '2')` and `b_z1` -> `('b', - 'noloss', '1')`. The charge is left as a string, as it is only written out. + 'noloss', '1')`. The charge stays a string because numba cannot parse one; + :func:`fragment_table` casts the collected column. """ idx = column.rfind("_") frag_type = column[:idx] @@ -268,7 +257,9 @@ def get_fragment_table( # noqa: PLR0913 Returns ------- pd.DataFrame - One row per kept fragment, in :class:`FragmentTableCols` columns. + One row per kept fragment, in :class:`FragmentTableCols` columns. `mz` and + `intensity` keep the dtype of the frame they came from; the charge and series + number are integers. """ frag_columns = fragment_mz_df.columns.to_numpy().astype("U") @@ -284,12 +275,12 @@ def get_fragment_table( # noqa: PLR0913 ) max_frag_mz = np.inf - frag_types = [] - frag_losses = [] - frag_charges = [] - frag_masses = [] - frag_intensities = [] - frag_numbers = [] + frag_types: list = [] + frag_losses: list = [] + frag_charges: list = [] + frag_numbers: list = [] + frag_masses: list = [] + frag_intensities: list = [] frag_idx_ranges = zip(frag_start_idx, frag_stop_idx) if verbose: frag_idx_ranges = tqdm.tqdm(frag_idx_ranges) @@ -323,27 +314,28 @@ def get_fragment_table( # noqa: PLR0913 infos = [_get_frag_info_from_column_name(column) for column in columns] types, losses, charges = zip(*infos) if infos else ((), (), ()) - frag_types.append(types) - frag_losses.append(losses) - frag_charges.append(charges) + frag_types.extend(types) + frag_losses.extend(losses) + frag_charges.extend(charges) + frag_numbers.extend(_get_frag_num(columns, rows, frag_len)) frag_masses.append(masses[idx_in_df]) frag_intensities.append(intens[idx_in_df]) - frag_numbers.append(_get_frag_num(columns, rows, frag_len)) - fragments_df = pd.DataFrame( + return pd.DataFrame( { - FragmentTableCols.PRECURSOR_ROW: np.arange(len(frag_start_idx)), + FragmentTableCols.PRECURSOR_ROW: np.repeat( + np.arange(len(frag_start_idx)), + [len(kept) for kept in frag_masses], + ), FragmentTableCols.FRAG_TYPE: frag_types, - FragmentTableCols.MZ: frag_masses, - FragmentTableCols.INTENSITY: frag_intensities, - FragmentTableCols.CHARGE: frag_charges, - FragmentTableCols.SERIES_NUMBER: frag_numbers, + FragmentTableCols.MZ: np.concatenate(frag_masses), + FragmentTableCols.INTENSITY: np.concatenate(frag_intensities), + FragmentTableCols.CHARGE: np.array(frag_charges, dtype=np.int64), + FragmentTableCols.SERIES_NUMBER: np.array(frag_numbers, dtype=np.int64), FragmentTableCols.LOSS_TYPE: frag_losses, } ) - fragments_df = explode_multiple_columns(fragments_df, FRAGMENT_VALUE_COLUMNS) - # a precursor that kept nothing explodes to one all-NaN row; drop those - return fragments_df.dropna(subset=[FragmentTableCols.MZ]) + def join_fragments( diff --git a/tests/unit/spectral_library/test_translate.py b/tests/unit/spectral_library/test_translate.py index 8f396448..72691faa 100644 --- a/tests/unit/spectral_library/test_translate.py +++ b/tests/unit/spectral_library/test_translate.py @@ -3,11 +3,7 @@ These tests pin the behaviour of `alphabase.spectral_library.translate` as it is today, so that the upcoming restructuring can be shown to change nothing. -One test still pins *buggy* behaviour that a later commit fixes, marked -`CHARACTERIZATION (bug)` in its docstring: the exploded fragment columns are -object dtype, with a string `FragmentCharge`. - -One more, `test_speclib_to_swath_df_returns_none`, pins a function that a later +One test, `test_speclib_to_swath_df_returns_none`, pins a function that a later commit removes rather than fixes. """ @@ -18,6 +14,7 @@ import pytest from alphabase.peptide.fragment import get_charged_frag_types +from alphabase.spectral_library import translate from alphabase.spectral_library.base import SpecLibBase from alphabase.spectral_library.reader import LibraryReaderBase from alphabase.spectral_library.translate import ( @@ -377,31 +374,26 @@ def test_disabled_mz_window_skips_empty_fragment_slots() -> None: assert with_loss <= {"SVIVSPYSTGAK", "LHDSTPPPYK"} -def test_exploded_fragment_columns_are_object_dtype() -> None: - """CHARACTERIZATION (bug): the fragment columns are objects, not numbers. +def test_fragment_columns_are_typed() -> None: + """The fragment columns are numbers, and m/z and intensity keep their own dtype. - Unlike the DIA-NN export, this one does no dtype casting, so `FragmentMz` and - friends are object-dtype, and `FragmentCharge` holds strings -- it is parsed - out of the fragment column name and never converted. That costs ~14x the - memory of the equivalent typed columns, and makes arithmetic on the charge - concatenate rather than add. The cells already hold correctly typed numpy - scalars, so a later commit types the columns from their source dtype, leaving - the written tsv byte-identical. + The fixture builds a float32 m/z frame and a float64 intensity frame, so the + two carry through independently rather than being cast to one width. """ - df = _export(_build_speclib()) + speclib = _build_speclib() + df = _export(speclib) - for column in ( - "FragmentType", - "FragmentMz", - "RelativeIntensity", - "FragmentCharge", - "FragmentNumber", - "FragmentLossType", - ): - assert df[column].dtype == object, column + assert df["FragmentMz"].dtype == speclib.fragment_mz_df.to_numpy().dtype + assert ( + df["RelativeIntensity"].dtype == speclib.fragment_intensity_df.to_numpy().dtype + ) + assert df["FragmentCharge"].dtype.kind == "i" + assert df["FragmentNumber"].dtype.kind == "i" + # the two label columns stay strings + assert df["FragmentType"].dtype == object + assert df["FragmentLossType"].dtype == object - assert set(map(type, df["FragmentCharge"])) == {str} - assert set(df["FragmentCharge"]) == {"1", "2"} + assert set(df["FragmentCharge"]) == {1, 2} def test_speclib_to_swath_df_returns_none() -> None: @@ -428,8 +420,6 @@ def test_translate_to_tsv_matches_the_in_memory_export(tmp_path) -> None: assert len(lines) == len(expected) + 1 written = pd.read_csv(tsv, sep="\t") assert list(written.columns) == list(expected.columns) - numeric = ["FragmentMz", "RelativeIntensity", "FragmentCharge", "FragmentNumber"] - expected[numeric] = expected[numeric].apply(pd.to_numeric) pd.testing.assert_frame_equal( written.reset_index(drop=True), expected.reset_index(drop=True), @@ -488,8 +478,6 @@ def test_translate_to_tsv_disabled_mz_window_matches_the_in_memory_export( assert (written["FragmentMz"] > 0).all() expected = _unfiltered(_build_speclib()) - numeric = ["FragmentMz", "RelativeIntensity", "FragmentCharge", "FragmentNumber"] - expected[numeric] = expected[numeric].apply(pd.to_numeric) pd.testing.assert_frame_equal( written.reset_index(drop=True), expected.reset_index(drop=True), @@ -520,3 +508,35 @@ def keys(df: pd.DataFrame) -> set: return set(zip(df["sequence"], df["mod_sites"], df["charge"].astype(int))) assert keys(reader.precursor_df) == keys(speclib.precursor_df) + + +class _DeadWriter: + """Stands in for a `WritingProcess` whose child died without writing.""" + + exitcode = 1 + + def __init__(self, task_queue, tsv) -> None: + pass + + def start(self) -> None: + pass + + def join(self) -> None: + pass + + +def test_translate_to_tsv_raises_when_the_writing_process_dies( + tmp_path, monkeypatch +) -> None: + """A writer that died is an error, not a quietly truncated file. + + `multiprocessing=True` is the default, and the writer process dies before it + writes anything if the calling script has no `if __name__ == "__main__":` + guard -- the norm on the spawn platforms, macOS and Windows. Nothing checked + on it, so the export printed its success message and returned normally, + leaving a 0-byte tsv behind. + """ + monkeypatch.setattr(translate, "WritingProcess", _DeadWriter) + + with pytest.raises(RuntimeError, match="exited with code 1"): + translate_to_tsv(_build_speclib(), str(tmp_path / "lib.tsv")) From 6c579268e9be7325b293a9d458efe78011372c6c Mon Sep 17 00:00:00 2001 From: Mohamed Sameh Date: Thu, 3 Sep 2026 00:50:22 +0200 Subject: [PATCH 4/5] refactor: name the SWATH export after the format it produces --- alphabase/spectral_library/translate.py | 39 ++++++++----------- nbs_tests/spectral_library/translate.ipynb | 8 ++-- tests/unit/spectral_library/test_translate.py | 20 ++++------ 3 files changed, 27 insertions(+), 40 deletions(-) diff --git a/alphabase/spectral_library/translate.py b/alphabase/spectral_library/translate.py index 3b281a57..3af25210 100644 --- a/alphabase/spectral_library/translate.py +++ b/alphabase/spectral_library/translate.py @@ -7,6 +7,7 @@ """ import multiprocessing as mp +import warnings import pandas as pd import tqdm @@ -44,8 +45,8 @@ "get_precursor_mz", # this module's own "WritingProcess", - "speclib_to_single_df", "speclib_to_swath_df", + "speclib_to_single_df", "translate_to_tsv", ] @@ -73,7 +74,7 @@ def _precursors_to_swath_df( # noqa: PLR0913 ) -> pd.DataFrame: """Convert precursor and fragment frames to a SWATH transition list. - The dataframe-in form of :func:`speclib_to_single_df`, so that one batch of + The dataframe-in form of :func:`speclib_to_swath_df`, so that one batch of precursors can be converted without standing up a `SpecLibBase` around it. """ df = pd.DataFrame() @@ -133,7 +134,7 @@ def _precursors_to_swath_df( # noqa: PLR0913 return df -def speclib_to_single_df( +def speclib_to_swath_df( speclib: SpecLibBase, *, translate_mod_dict: dict = None, @@ -145,10 +146,9 @@ def speclib_to_single_df( modloss: str = "H3PO4", verbose=True, ) -> pd.DataFrame: - """Convert an alphabase library into a SWATH/Spectronaut transition list. + """Convert an alphabase library to a SWATH/Spectronaut transition-list dataframe. - The in-memory counterpart of :func:`translate_to_tsv`, which writes the same - table to a file. + :func:`translate_to_tsv` writes the same table to a file. Parameters ---------- @@ -167,7 +167,7 @@ def speclib_to_single_df( Returns ------- pd.DataFrame - a single dataframe in the SWATH-like format + One row per precursor/fragment pair, in SWATH column names. """ return _precursors_to_swath_df( @@ -185,22 +185,15 @@ def speclib_to_single_df( ) -def speclib_to_swath_df( - speclib: SpecLibBase, - *, - keep_k_highest_fragments: int = 12, - min_frag_mz=200, - max_frag_mz=2000, - min_frag_intensity=0.01, -) -> pd.DataFrame: - speclib_to_single_df( - speclib, - translate_mod_dict=None, - keep_k_highest_fragments=keep_k_highest_fragments, - min_frag_mz=min_frag_mz, - max_frag_mz=max_frag_mz, - min_frag_intensity=min_frag_intensity, +def speclib_to_single_df(speclib: SpecLibBase, **kwargs) -> pd.DataFrame: + """Deprecated alias of :func:`speclib_to_swath_df`.""" + warnings.warn( + "speclib_to_single_df is deprecated; use speclib_to_swath_df, which names the " + "format it produces.", + FutureWarning, + stacklevel=2, ) + return speclib_to_swath_df(speclib, **kwargs) class WritingProcess(mp.Process): @@ -248,7 +241,7 @@ def translate_to_tsv( Fragment m/z range; fragments outside it are dropped. Pass 0 for no lower bound and `np.inf` for no upper bound. - See :func:`speclib_to_single_df`, whose parameters this shares, for the rest. + See :func:`speclib_to_swath_df`, whose parameters this shares, for the rest. """ if multiprocessing: queue_size = 1000000 // batch_size diff --git a/nbs_tests/spectral_library/translate.ipynb b/nbs_tests/spectral_library/translate.ipynb index 99414857..47c91b7e 100644 --- a/nbs_tests/spectral_library/translate.ipynb +++ b/nbs_tests/spectral_library/translate.ipynb @@ -33,7 +33,7 @@ "import pandas as pd\n", "\n", "from alphabase.spectral_library.base import SpecLibBase\n", - "from alphabase.spectral_library.translate import create_modified_sequence, speclib_to_single_df, translate_to_tsv" + "from alphabase.spectral_library.translate import create_modified_sequence, speclib_to_swath_df, translate_to_tsv" ] }, { @@ -538,10 +538,10 @@ "spec_lib._precursor_df = precursor_df\n", "spec_lib._fragment_intensity_df = frag_mass_df.copy()\n", "spec_lib._fragment_mz_df = frag_mass_df.copy()\n", - "df = speclib_to_single_df(spec_lib, min_frag_mz=300, max_frag_mz=1800)\n", + "df = speclib_to_swath_df(spec_lib, min_frag_mz=300, max_frag_mz=1800)\n", "assert (df.FragmentMz>=300).all()\n", "assert (df.FragmentMz<=1800).all()\n", - "df = speclib_to_single_df(spec_lib, min_frag_mz=200, min_frag_nAA=3)\n", + "df = speclib_to_swath_df(spec_lib, min_frag_mz=200, min_frag_nAA=3)\n", "assert (df.FragmentNumber>=3).all()" ] }, @@ -846,7 +846,7 @@ "spec_lib._precursor_df = precursor_df\n", "spec_lib._fragment_intensity_df = frag_mass_df.copy()\n", "spec_lib._fragment_mz_df = frag_mass_df.copy()\n", - "speclib_sdf = speclib_to_single_df(spec_lib)\n", + "speclib_sdf = speclib_to_swath_df(spec_lib)\n", "with tempfile.TemporaryFile('w+') as f:\n", " translate_to_tsv(spec_lib, f, batch_size=2, multiprocessing=False)\n", " f.seek(0)\n", diff --git a/tests/unit/spectral_library/test_translate.py b/tests/unit/spectral_library/test_translate.py index 72691faa..4f0ad135 100644 --- a/tests/unit/spectral_library/test_translate.py +++ b/tests/unit/spectral_library/test_translate.py @@ -2,9 +2,6 @@ These tests pin the behaviour of `alphabase.spectral_library.translate` as it is today, so that the upcoming restructuring can be shown to change nothing. - -One test, `test_speclib_to_swath_df_returns_none`, pins a function that a later -commit removes rather than fixes. """ import hashlib @@ -95,7 +92,7 @@ def _build_speclib(**extra_columns) -> SpecLibBase: def _export(speclib: SpecLibBase, **kwargs) -> pd.DataFrame: """Run the in-memory export with the progress bar off.""" - return speclib_to_single_df(speclib, verbose=False, **kwargs) + return speclib_to_swath_df(speclib, verbose=False, **kwargs) def _unfiltered(speclib: SpecLibBase, **kwargs) -> pd.DataFrame: @@ -396,15 +393,12 @@ def test_fragment_columns_are_typed() -> None: assert set(df["FragmentCharge"]) == {1, 2} -def test_speclib_to_swath_df_returns_none() -> None: - """CHARACTERIZATION: the wrapper discards its result, returning None. +def test_speclib_to_single_df_is_a_deprecated_alias() -> None: + """The old name still works, warns, and returns what the new one returns.""" + with pytest.warns(FutureWarning, match="use speclib_to_swath_df"): + aliased = speclib_to_single_df(_build_speclib(), verbose=False) - It is a parameter subset of `speclib_to_single_df` that has been missing its - `return` since 93aa2fa, so it is only ever called for the side effect of - editing the library. Pinned as the evidence that removing it later is safe: - nothing can depend on a function that has only ever returned None. - """ - assert speclib_to_swath_df(_build_speclib()) is None + pd.testing.assert_frame_equal(aliased, _export(_build_speclib())) def test_translate_to_tsv_matches_the_in_memory_export(tmp_path) -> None: @@ -461,7 +455,7 @@ def test_translate_to_tsv_disabled_mz_window_matches_the_in_memory_export( `translate_to_tsv` used to mask by m/z without checking whether the window was disabled, so 0/0 -- the documented way to turn the filter off -- zeroed every real fragment and wrote a file of nothing but empty slots, while - `speclib_to_single_df` given the same arguments kept the real ones. + `speclib_to_swath_df` given the same arguments kept the real ones. """ tsv = str(tmp_path / "lib.tsv") translate_to_tsv( From e9db55a98321f05603922a7a5c97ec650ec4181a Mon Sep 17 00:00:00 2001 From: Mohamed Kotb Date: Thu, 17 Sep 2026 09:13:33 +0200 Subject: [PATCH 5/5] apply comments --- alphabase/spectral_library/translate_core.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/alphabase/spectral_library/translate_core.py b/alphabase/spectral_library/translate_core.py index 642899ac..f9ed6fca 100644 --- a/alphabase/spectral_library/translate_core.py +++ b/alphabase/spectral_library/translate_core.py @@ -330,14 +330,13 @@ def get_fragment_table( # noqa: PLR0913 FragmentTableCols.FRAG_TYPE: frag_types, FragmentTableCols.MZ: np.concatenate(frag_masses), FragmentTableCols.INTENSITY: np.concatenate(frag_intensities), - FragmentTableCols.CHARGE: np.array(frag_charges, dtype=np.int64), + FragmentTableCols.CHARGE: np.array(frag_charges, dtype=np.int8), FragmentTableCols.SERIES_NUMBER: np.array(frag_numbers, dtype=np.int64), FragmentTableCols.LOSS_TYPE: frag_losses, } ) - def join_fragments( precursor_df: pd.DataFrame, fragment_df: pd.DataFrame,