move fragment types to constant #263
Conversation
|
This currently fails because the pinned version |
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
| "y_modloss": FragmentType( | ||
| name="y_modloss", | ||
| ref_ion="y", | ||
| formula="N(-1)H(-2)", |
| "c_lossH": "b+N(1)H(2)", | ||
| "z_addH": "y+N(-1)H(-1)", | ||
|
|
||
| class DIRECTION: |
There was a problem hiding this comment.
Direction (also LOSS, LOSS_INVERSE.. )
| @@ -66,11 +302,20 @@ def parse_all_frag_type_representation(): | |||
| parse_all_frag_type_representation() | |||
|
|
|||
|
|
|||
| def sort_charged_frag_types(charged_frag_types: List[str]) -> List[str]: | |||
| """charged frag types are sorted by (no-loss, loss) and then alphabetically""" | |||
| has_loss = [f.count("_") > 1 for f in charged_frag_types] | |||
There was a problem hiding this comment.
has_loss = [f.replace(FRAGMENT_CHARGE_SEPARATOR, "").count("_") > 0 for f in charged_frag_types]
| @@ -93,9 +338,12 @@ def get_charged_frag_types( | |||
| """ | |||
| charged_frag_types = [] | |||
| for _type in frag_types: | |||
There was a problem hiding this comment.
_type->frag_type, _ch -> charge?
|
|
||
| if FRAGMENT_CHARGE_SEPARATOR in charged_frag_type: | ||
| _type, _ch = charged_frag_type.split(FRAGMENT_CHARGE_SEPARATOR) | ||
| return _type, int(_ch) |
There was a problem hiding this comment.
here you could raise if _ch is not int
| return _type, int(_ch) | ||
|
|
||
| if FRAGMENT_CHARGE_SEPARATOR in charged_frag_type: | ||
| _type, _ch = charged_frag_type.split(FRAGMENT_CHARGE_SEPARATOR) |
There was a problem hiding this comment.
_type->frag_type, _ch -> charge?
| else: | ||
| frag_directions.append(0) | ||
| for charged_frag_type in fragment_mz_df.columns.values: | ||
| frag_type, charge = parse_charged_frag_type(charged_frag_type) |
| for charged_frag_type in fragment_mz_df.columns.values: | ||
| frag_type, charge = parse_charged_frag_type(charged_frag_type) | ||
| frag_charges.append(charge) | ||
| frag_types.append(FRAGMENT_TYPES[frag_type].series) |
There was a problem hiding this comment.
the new implementation really offers enhanced maintainability and readability!
|
|
||
| import pandas as pd | ||
| import tqdm | ||
|
|
||
|
|
||
| # Create a warning class for deprecation |
There was a problem hiding this comment.
If I could improve only one thing in code generated by LLMs, it would be "drop this comment describing what the next line of code does".
|
|
||
| # because we dont know the loss, we assume every loss type is phospho | ||
| class LOSS: | ||
| MODLOSS = 98 |
There was a problem hiding this comment.
maybe give some context here what these numbers mean?
| class LOSS: | ||
| MODLOSS = 98 | ||
| H2O = 18 | ||
| NH3 = 17 | ||
| LOSSH = 1 | ||
| ADDH = 2 | ||
| NONE = 0 | ||
|
|
||
|
|
||
| LOSS_INVERSE = { | ||
| 18: "H2O", | ||
| 17: "NH3", | ||
| 98: "modloss", | ||
| 1: "lossH", | ||
| 0: "", | ||
| 2: "addH", | ||
| } |
There was a problem hiding this comment.
Consider using this idiom:
class Losses:
"""String contants defining losses.""
H2O = "H2O"
...
LOSS_MAPPING = {
"""Mapping of loss names to ... .""
Losses.H2O: 18,
...
}
LOSS_MAPPING_INV = { v : k for k, v in LOSS_MAPPING.items() }
(same for SERIES)
So far the definition of what is a valid fragment type was somewhat loose.
This PR:
FRAGMENT_TYPESwhich allows checking if a type is supported