[translate 2/3] Split the shared export machinery out of translate.py - #445
Draft
mo-sameh wants to merge 5 commits into
Draft
[translate 2/3] Split the shared export machinery out of translate.py#445mo-sameh wants to merge 5 commits into
mo-sameh wants to merge 5 commits into
Conversation
Clear the ground before restructuring, so the moves in the following commits are readable. Output is unchanged. - the commented-out `df.explode` fallback in `merge_precursor_fragment_df`, superseded by `explode_multiple_columns`, which handles the same old-pandas case it was guarding against - two commented-out column assignments, `LabelModifiedSequence` and `ProteinGroups` - `DiannParquetCols.SIGNATURE`, declared but never written; it is referenced only by the docstring saying DIA-NN wants it omitted, which stays - the six `frag_*_head` arguments of `speclib_to_single_df`, which only passed their own defaults through to `merge_precursor_fragment_df`. No caller passes them, in alphabase, its notebooks, peptdeep or alphadia; the parameters on `merge_precursor_fragment_df` itself stay, since the DIA-NN export uses them. - the pandas-version branch picking `to_csv`'s newline argument, duplicated in `WritingProcess.run` and `translate_to_tsv`, hoisted to one module constant. Kept rather than dropped: `requirements_loose.txt` does not pin pandas, so a pre-1.5 install is still possible. Verified with a scratch harness comparing 15 outputs -- 7 transition-list parameter combinations, 4 DIA-NN combinations, 2 tsv batchings and 2 parquet batchings -- against the same functions loaded from main: all identical, frames compared including dtypes and files by digest. Test files are untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`merge_precursor_fragment_df` took the half-built *output* frame, so both formats copied `frag_start_idx`/`frag_stop_idx` into it just for the flattener to read and then dropped them again, and the DIA-NN export smuggled a precursor pointer through it to flag base peaks. Split it in two: `fragment_table` takes the index arrays and returns one row per kept fragment in canonical columns plus `precursor_row`, and `join_fragments` repeats each precursor row across its fragments. Each format now names the fragment columns with a rename dict instead of `frag_*_head` arguments, so a third dialect is a dict literal. Output is unchanged. `fragment_table` still ends in `explode_multiple_columns` on purpose: building the columns from typed arrays would change the exported dtypes, which belongs to the fixes PR. `merge_precursor_fragment_df` is removed; no caller exists in alphabase, peptdeep or alphadia. `DIANN_PARQUET_FRAG_HEADS` is renamed to `DIANN_FRAGMENT_COLUMNS`, and is in no released tag. Verified with the scratch harness: 15 outputs identical to main, compared including dtypes and index. Test files are untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The m/z window zeroed intensities in the caller's library instead of dropping fragments, so an export edited what it was handed, exporting twice at different windows was order-dependent, and empty fragment slots (m/z 0) leaked into the output when the window was disabled -- `translate_to_tsv` at 0/0 wrote nothing else. Filter inside `fragment_table` on the per-precursor copy it already made, and read precursor m/z through `get_precursor_mz` instead of writing it onto the caller's frame. `mask_fragment_intensity_by_mz_` and `mask_fragment_intensity_by_frag_nAA` go with the design they implemented. An unbounded window is now expressed by its own bounds, 0 and `np.inf`, rather than the 0/0 sentinel, which warns and is treated as unbounded. Output is unchanged except where the window is disabled: of 18 outputs compared against main, the four that differ are exactly those calls. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mo-sameh
changed the title
mo-sameh
changed the title
mschwoer
reviewed
Sep 9, 2026
Comment on lines
+9
to
+10
| Before this module, they lived in ``translate.py``, which made the SWATH format the de | ||
| facto shared library: ``translate_diann`` imported five helpers from it. |
Contributor
There was a problem hiding this comment.
please remove references to older code
| return default | ||
|
|
||
|
|
||
| # @numba.njit #(cannot use numba for pd.Series) |
Contributor
There was a problem hiding this comment.
which argument is the series?
| library's fragment frames are left exactly as they were. Fragments outside the m/z | ||
| window are dropped rather than zeroed, as are empty fragment slots -- a `*_modloss` | ||
| column of a precursor whose modification has no loss carries m/z 0, and selecting it | ||
| would export a fragment that does not exist. An unbounded window is expressed by the |
Contributor
There was a problem hiding this comment.
Please rephrase the last sentence
Contributor
There was a problem hiding this comment.
in general, docstrings are very LLM-ish, might be worth a human overhaul :-)
| frag_masses = [] | ||
| frag_intensities = [] | ||
| frag_numbers = [] | ||
| iters = zip(frag_start_idx, frag_stop_idx) |
Contributor
There was a problem hiding this comment.
frag_start_stop_idx_iterator?
| iters = tqdm.tqdm(iters) | ||
| for start, end in iters: | ||
| masses = fragment_mz_df.iloc[start:end, :].to_numpy() | ||
| keep = (masses > 0) & (masses >= min_frag_mz) & (masses <= max_frag_mz) |
| ] | ||
|
|
||
|
|
||
| def fragment_table( # noqa: PLR0913 |
| frag_intensities.append(intens[idx_in_df]) | ||
| frag_numbers.append(_get_frag_num(columns, rows, frag_len)) | ||
|
|
||
| table = pd.DataFrame( |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Goal
translate_diann.pyimported five helpers fromtranslate.py, which made the SWATH format the de facto shared library for the DIA-NN one. Move what both formats share into a newtranslate_core.pyso neither depends on the other's dialect.What's here
translate_core.pyholds the modified-sequence rendering, the candidate precursor columns, and the fragment flattening. Each export keeps only its own column names and writer, and renames the canonical fragment columns into its own dialect through a dict.The flattening is where the real change is. It used to filter by zeroing intensities in the caller's library and letting a later
min_frag_intensitydrop the rows. One design choice, four symptoms: a default export silently zeroed 60% of the library's intensities, a second export at a wider window returned different fragments than a fresh one, the export wrote aprecursor_mzcolumn into the caller's frame, and disabling the m/z window exported empty fragment slots at m/z 0. Fragments are now selected on a per-precursor copy and dropped rather than zeroed, which fixes all four.Verification
Every export was run against
main's modules over 19 configurations and compared including dtypes and index. The first four commits are byte-identical tomainon all 19, with no test edits. The fifth differs on exactly the four cases that disable the m/z window — the fix — and the characterization tests that pinned the old behaviour change with it.pytestwarnings drop from 41 to 7; theSettingWithCopyWarningstorm was the export writing into batch slices of the caller's library.