Repository navigation
Commit 6707838
[3/5] Refactor: extract the charged fragment type annotation and the pointer reannotation (#436)
* Characterize flatten_fragments before the memory refactor
flatten_fragments had no unit test. Add black-box tests that pin its contract, so
the refactoring that follows can be checked step by step.
`_dense_library` builds dense frames with mz == 0 padding from both sources that
produce it: unmodified precursors carry no modloss fragments, and some fragments
fall outside the mz range. Intensities are distinct, so the top-k selection is
unambiguous.
`_expected_keep_mask` derives the expected mask per precursor, without
flatten_fragments. The tests cover the kept fragments over five combinations of
`keep_top_k_fragments` and `min_fragment_intensity`, every annotation column, the
reannotated precursor pointers, `custom_df` and `custom_columns`, the path
without intensities, an empty library, and a precursor long enough to need more
than a uint8 fragment number.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Make the fragment parsing helpers private
`parse_fragment`, `fill_in_indices` and `calculate_fragment_numbers` only serve
`flatten_fragments`, and nothing in alphabase, alphadia or peptdeep calls them.
A leading underscore states that, and frees their signatures for the refactoring
that follows.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Extract the charged fragment type annotation and the pointer reannotation
Move two concerns out of the body of `flatten_fragments`: the per-column
annotation of the charged fragment types, and the reannotation of the precursor
pointers. The function now reads as its steps, and the memory refactoring that
follows changes a named helper instead of a long function body.
`_annotate_charged_frag_types` returns typed arrays rather than four parallel
lists of Python ints. Tiling a typed array keeps its dtype, so the dense
type/loss_type/charge/direction arrays no longer pass through an int64
intermediate of 8 bytes per dense slot.
No functional change. Output stays bit-identical to main across 5400 generated
configurations of library shape, padding, top-k, intensity threshold and
custom_columns.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Pin flatten_fragments against a hand-written library
_expected_keep_mask reimplemented the filtering logic, so it could agree
with a broken flatten_fragments. Add a minimal_library fixture of two
precursors over a forward and a reverse fragment type, with the kept
slots written out per filter setting, and assert both the helper and
flatten_fragments against it.
Derive the long precursor row count from the uint8 boundary it tests
instead of picking a number near the fill_in_indices limit, and guard
the limit explicitly so exceeding it fails on an assertion rather than
on a broadcast error inside numba.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Document the fragment numbering convention
"fragment number" read as ambiguous between a count and an enumeration.
Spell out that it is the MS series numbering, and contrast it with the
fragment position that all series share.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Assert against a hand-written library
The library was generated from a seeded rng and the expectations came
from _expected_keep_mask, which reimplemented the filtering logic and so
could agree with a broken flatten_fragments.
Replace both with one library fixture over hand-written mz and intensity
grids, and hand-written keep masks per filter setting. Two fragment types
carry the whole annotation surface: y_z1 is reverse without loss and
b_modloss_z1 is forward with loss, so direction, series and loss all vary
over two columns.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Assert the flat fragment frames with assert_frame_equal
Comparing column by column left the column set, the column order and the
dtypes unchecked. Assert the whole annotated frame against an expected
frame instead, and fold the row count of the filter cases into a frame
comparison of mz and intensity.
type and charge come out as int8, so the expected frame spells that out.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Fold the repeated annotation tiling into a loop
The three tile calls differed only in column name and source array, and
the comment above them explained the dtype of the previous implementation
rather than the code it sat on. Loop over the columns instead; the dtypes
stay visible where they are set, in _annotate_charged_frag_types.
Trim the two comments carried into the extracted helpers. The direction
comment claimed a third value of 0 that DIRECTION_MAPPING never produces.
Column order of the flat fragment dataframe is unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Point the test at the private helper name
The merged tests named fill_in_indices, which this branch renamed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Assert every flat frame with assert_frame_equal
Replace the remaining column comparisons. Each one now also pins the
column set, the column order and the dtypes:
- without_intensity subsumes its separate missing-column assertion
- selects_custom_columns checked only the column names
- filters_custom_df_columns now pins where cardinality lands
- reannotates_precursor_pointers compares hardcoded flat pointers
instead of reslicing mz per precursor, which was the last place the
tests recomputed an expectation
- empty_precursor_df compares both frames instead of their lengths
Five tests assert against the same unfiltered library, so it moves into
a flat_unfiltered fixture.
test_flatten_fragments_long_precursor keeps its property assertions. It
builds 257 rows to push fragment numbers past 255, so a frame of 1542
generated values would bury what it tests.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Compare the long precursor maxima and dtypes as series
The four scalar assertions become two equality comparisons. The dtypes
need their own comparison, because .max() over columns of different
dtypes upcasts to a common one and would hide a narrowed column.
Also drop the tuple rebuild in the parse test, expected is already the
tuple being compared.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Merge refactor/private-fragment-parsing-helpers into refactor/annotate-charged-frag-types
Brings main in behind it. The merge was textually clean but left the
module unimportable: _annotate_charged_frag_types is annotated
-> Tuple[...], and main dropped Python 3.8 and with it the typing
import. Changed to the builtin tuple, matching the rest of the file.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: GeorgWa <georg@aplusia.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>1 parent 52e5821 commit 6707838
1 file changed
Lines changed: 83 additions & 44 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
980 | 980 | | |
981 | 981 | | |
982 | 982 | | |
| 983 | + | |
| 984 | + | |
| 985 | + | |
| 986 | + | |
| 987 | + | |
| 988 | + | |
| 989 | + | |
| 990 | + | |
| 991 | + | |
| 992 | + | |
| 993 | + | |
| 994 | + | |
| 995 | + | |
| 996 | + | |
| 997 | + | |
| 998 | + | |
| 999 | + | |
| 1000 | + | |
| 1001 | + | |
| 1002 | + | |
| 1003 | + | |
| 1004 | + | |
| 1005 | + | |
| 1006 | + | |
| 1007 | + | |
| 1008 | + | |
| 1009 | + | |
| 1010 | + | |
| 1011 | + | |
| 1012 | + | |
| 1013 | + | |
| 1014 | + | |
| 1015 | + | |
| 1016 | + | |
| 1017 | + | |
| 1018 | + | |
| 1019 | + | |
| 1020 | + | |
| 1021 | + | |
| 1022 | + | |
| 1023 | + | |
| 1024 | + | |
| 1025 | + | |
| 1026 | + | |
| 1027 | + | |
| 1028 | + | |
| 1029 | + | |
| 1030 | + | |
| 1031 | + | |
| 1032 | + | |
| 1033 | + | |
| 1034 | + | |
| 1035 | + | |
| 1036 | + | |
| 1037 | + | |
| 1038 | + | |
| 1039 | + | |
| 1040 | + | |
| 1041 | + | |
| 1042 | + | |
| 1043 | + | |
| 1044 | + | |
| 1045 | + | |
| 1046 | + | |
| 1047 | + | |
| 1048 | + | |
| 1049 | + | |
| 1050 | + | |
| 1051 | + | |
983 | 1052 | | |
984 | 1053 | | |
985 | 1054 | | |
| |||
1060 | 1129 | | |
1061 | 1130 | | |
1062 | 1131 | | |
1063 | | - | |
1064 | | - | |
1065 | | - | |
1066 | | - | |
| 1132 | + | |
| 1133 | + | |
| 1134 | + | |
1067 | 1135 | | |
1068 | | - | |
1069 | | - | |
1070 | | - | |
1071 | | - | |
1072 | | - | |
1073 | | - | |
1074 | | - | |
1075 | | - | |
1076 | | - | |
1077 | | - | |
1078 | | - | |
1079 | | - | |
1080 | | - | |
1081 | | - | |
1082 | | - | |
1083 | | - | |
1084 | | - | |
1085 | | - | |
1086 | | - | |
| 1136 | + | |
| 1137 | + | |
| 1138 | + | |
| 1139 | + | |
| 1140 | + | |
| 1141 | + | |
| 1142 | + | |
| 1143 | + | |
1087 | 1144 | | |
1088 | | - | |
1089 | | - | |
1090 | | - | |
| 1145 | + | |
1091 | 1146 | | |
1092 | 1147 | | |
1093 | | - | |
| 1148 | + | |
1094 | 1149 | | |
1095 | 1150 | | |
1096 | 1151 | | |
| |||
1104 | 1159 | | |
1105 | 1160 | | |
1106 | 1161 | | |
1107 | | - | |
1108 | | - | |
1109 | | - | |
1110 | | - | |
1111 | | - | |
1112 | | - | |
1113 | 1162 | | |
1114 | 1163 | | |
1115 | 1164 | | |
| |||
1125 | 1174 | | |
1126 | 1175 | | |
1127 | 1176 | | |
1128 | | - | |
1129 | | - | |
1130 | | - | |
1131 | | - | |
1132 | | - | |
1133 | | - | |
1134 | | - | |
1135 | | - | |
1136 | | - | |
1137 | | - | |
1138 | | - | |
| 1177 | + | |
1139 | 1178 | | |
1140 | 1179 | | |
1141 | 1180 | | |
| |||
0 commit comments