[translate 3/3] Fix correctness and naming in the spectral library exports - #446
Draft
mo-sameh wants to merge 4 commits into
Draft
[translate 3/3] Fix correctness and naming in the spectral library exports#446mo-sameh wants to merge 4 commits into
mo-sameh wants to merge 4 commits into
Conversation
`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.
`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) <noreply@anthropic.com>
mo-sameh
changed the title
mo-sameh
changed the title
mschwoer
reviewed
Sep 9, 2026
| 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), |
Contributor
There was a problem hiding this comment.
is int64 required for charges?
| frag_types.append(types) | ||
| frag_losses.append(losses) | ||
| frag_charges.append(charges) | ||
| frag_types.extend(types) |
Contributor
There was a problem hiding this comment.
we change from append to extend, is this intended?
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
Fix the remaining defects the characterization tests exposed, now that the structure makes them one-line changes, and give the exports names that say what they produce.
What's here
rt_norm_predis accepted as a retention time. peptdeep writes it alongsidert_pred, so a library whosert_predwas dropped was rejected outright.Flagsmarks one base peak per precursor row. It grouped onPrecursor.Id, which is not unique —SpecLibBase.appendproduces libraries holding the same precursor twice, and those shared a single flag. Two appended libraries: 8 flags for 16 precursor rows, now 16.object, andFragmentChargeheld strings, so arithmetic on it concatenated. The written tsv and parquet are byte-identical; only the in-memory frames change.translate_to_tsvnotices a dead writer.multiprocessing=Trueis the default, and that process dies before writing anything if the calling script has noif __name__ == "__main__":guard — the norm on macOS and Windows. The export printed its success message and returned over a 0-byte file.speclib_to_single_dfis nowspeclib_to_swath_df. The good name already existed as a wrapper that has been missing itsreturnsince the module arrived from AlphaPeptDeep in 2022, so it only ever returnedNone. The real function takes the name; the old one stays as a deprecated alias. Nothing breaks — the wrapper's arguments are a subset, under the same names and defaults.The module now reads
speclib_to_*_dfto build a frame,translate_to_*to write a file.