Refactor readers iv #246
Refactor readers iv #246
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
| "[Phospho]", | ||
| "_(Phospho)", | ||
| "_[Phospho]", | ||
| ] |
There was a problem hiding this comment.
@jalew188 just by symmetry, I would expect (Phospho) here too?
There was a problem hiding this comment.
It should be, but I am not sure what is this testing for.
There was a problem hiding this comment.
changed the test data to be more realistic here: https://github.com/MannLabs/alphabase/pull/247/files#diff-49c8564dc5623dcc26f0b191744d709f762671f9d6cad81442985f3f75cb23fa
There was a problem hiding this comment.
Make sense. It looks like () processing is missing here .
There was a problem hiding this comment.
Maybe currently, this is not used by any readers, but we should add () case
| @@ -0,0 +1,113 @@ | |||
| """Utility functions for PSM readers.""" | |||
| """ | ||
|
|
||
| _reader_type = "spectronaut" | ||
| _add_unimod_to_mod_mapping = True |
There was a problem hiding this comment.
should we also add this to the base class?
| self.rev_mod_mapping = self._get_reversed_mod_mapping() | ||
|
|
||
| def _init_modification_mapping(self) -> None: | ||
| self.modification_mapping = {} | ||
|
|
||
| def _add_all_unimod(self) -> None: | ||
| for mod_name, unimod in MOD_TO_UNIMOD_DICT.items(): | ||
| if mod_name in self.modification_mapping: |
There was a problem hiding this comment.
could we reuse the logic used to add custom mods here? I feel like adding unimod, adding reader specific mods and adding custom mods is very similar.
| else: | ||
| self.modification_mapping[mod_name] = [unimod] | ||
|
|
||
| def _extend_mod_brackets(self) -> None: |
There was a problem hiding this comment.
This code (ab in general, not your PR) feels to me like we should start a ModificationMapping class 😆
There was a problem hiding this comment.
Agree. It also depends on how heavy we are going to use these reader classes. Before, I just used them to get unified training data.
| @@ -457,20 +428,9 @@ def _translate_columns(self, origin_df: pd.DataFrame) -> None: | |||
| self._psm_df[PsmDfCols.SPEC_IDX] = self._psm_df[PsmDfCols.SCAN_NUM] - 1 | |||
|
|
|||
| def _transform_table(self) -> None: # noqa: B027 empty method in an abstract base class | |||
| """Transform the dataframe format if needed. | |||
| """Transform the dataframe format if needed, ddd information inplace into self._psm_df. | |||
Trying to simplify the inheritance structure
self.csv_sepa ordinary variable