-
Notifications
You must be signed in to change notification settings - Fork 4
Update changes in PTM diann handling #59
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 13 commits
96d2fe1
232e9ec
65fbeb8
1a7b394
c66ff69
1b54714
4557f07
b7cdfe7
5f04fe4
d76f991
9feb130
08715f3
a88d479
6e14a1b
5008129
51f8610
2a03fcc
4c436ad
17ee180
742695d
88989e2
af8aa49
cd28956
a1dc5ac
465f606
78c988d
2deda03
e5310ef
372dbeb
106abbf
00008df
7adceb5
6255ef9
a9251f7
4631f79
c82a93f
8aea86b
9fdb40c
1b0fe77
4128e98
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| __version__ = "0.0.23" | ||
| __version__ = "0.0.24" |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -7,12 +7,13 @@ | |||||||||||||||||||||||
| import logging | ||||||||||||||||||||||||
| import re | ||||||||||||||||||||||||
| from typing import List, Tuple | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| from collections import defaultdict | ||||||||||||||||||||||||
| import click | ||||||||||||||||||||||||
| from sdrf_pipelines.openms.unimod import UnimodDatabase | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| logging.basicConfig(format="%(asctime)s [%(funcName)s] - %(message)s", level=logging.DEBUG) | ||||||||||||||||||||||||
| logger = logging.getLogger(__name__) | ||||||||||||||||||||||||
| unimod_database = UnimodDatabase() | ||||||||||||||||||||||||
ypriverol marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
coderabbitai[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| @click.command("dianncfg", short_help="Create DIA-NN config file with enzyme and PTMs") | ||||||||||||||||||||||||
|
|
@@ -32,8 +33,7 @@ def dianncfg(ctx, enzyme, fix_mod, var_mod): | |||||||||||||||||||||||
| :param var_mod: A string of variable modifications, separated by commas. | ||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||
| cut = enzyme_cut(enzyme) | ||||||||||||||||||||||||
| unimod_database = UnimodDatabase() | ||||||||||||||||||||||||
| fix_ptm, var_ptm = convert_mod(unimod_database, fix_mod, var_mod) | ||||||||||||||||||||||||
| fix_ptm, var_ptm = convert_mod(fix_mod, var_mod) | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| var_ptm_str = " --var-mod " | ||||||||||||||||||||||||
| fix_ptm_str = " --fixed-mod " | ||||||||||||||||||||||||
|
|
@@ -42,83 +42,97 @@ def dianncfg(ctx, enzyme, fix_mod, var_mod): | |||||||||||||||||||||||
| for mod in fix_ptm: | ||||||||||||||||||||||||
| diann_fix_ptm += fix_ptm_str + mod | ||||||||||||||||||||||||
| for mod in var_ptm: | ||||||||||||||||||||||||
| diann_var_ptm += var_ptm_str + mod | ||||||||||||||||||||||||
| if mod == "UniMod:765,-131.040485,*nM": | ||||||||||||||||||||||||
ypriverol marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||||
| diann_var_ptm += " --met-excision " | ||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||
| diann_var_ptm += var_ptm_str + mod | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| with open("diann_config.cfg", "w") as file: | ||||||||||||||||||||||||
| file.write("--cut " + cut + diann_fix_ptm + diann_var_ptm) | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| def convert_mod(unimod_database, fix_mod: str, var_mod: str) -> Tuple[List, List]: | ||||||||||||||||||||||||
| def get_mod(mod, mod_type): | ||||||||||||||||||||||||
daichengxin marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||
| pattern = re.compile(r"\((.*?)\)") | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
| tag = 0 | ||||||||||||||||||||||||
| diann_mod_accession = None | ||||||||||||||||||||||||
| diann_mod_name = None | ||||||||||||||||||||||||
| for modification in unimod_database.modifications: | ||||||||||||||||||||||||
| if modification.get_name() == mod.split(" ")[0]: | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
| diann_mod_accession = modification.get_accession().replace("UNIMOD:", "UniMod:") + "," + str(modification._delta_mono_mass) | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
| diann_mod_accession = modification.get_accession().replace("UNIMOD:", "UniMod:") + "," + str(modification._delta_mono_mass) | |
| # Prefer public accessor for mono mass if available; fallback to private attribute with documentation. | |
| try: | |
| # Try to use a public method/property if available | |
| mono_mass = modification.get_delta_mono_mass() | |
| except AttributeError: | |
| # No public accessor; document why direct access is necessary | |
| # Direct access to _delta_mono_mass is required as no public method is available. | |
| # This may be fragile if the underlying library changes. | |
| mono_mass = modification._delta_mono_mass | |
| diann_mod_accession = modification.get_accession().replace("UNIMOD:", "UniMod:") + "," + str(mono_mass) |
daichengxin marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
ypriverol marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Copilot
AI
Nov 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using exit(1) directly is not ideal in library code as it terminates the entire Python process. Consider raising a custom exception (e.g., ValueError or a custom ModificationNotSupportedError) instead, allowing callers to handle the error appropriately. This pattern appears in lines 71, 85, and 110.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot open a new pull request to apply changes based on this feedback
Copilot
AI
Nov 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using exit(1) directly is not a best practice in library code, as it terminates the entire Python process and prevents calling code from handling the error. Consider raising an appropriate exception (e.g., ValueError or a custom exception) instead, allowing the CLI command handler to catch it and exit with the appropriate code. This pattern applies to all exit(1) calls in this file (lines 104, 118, 125).
ypriverol marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Copilot
AI
Nov 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using exit(1) directly is not ideal in library code as it terminates the entire Python process. Consider raising a custom exception (e.g., ValueError or a custom ModificationNotSupportedError) instead, allowing callers to handle the error appropriately.
ypriverol marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Copilot
AI
Nov 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using exit(1) directly is not ideal in library code as it terminates the entire Python process. Consider raising a custom exception (e.g., ValueError or a custom ModificationNotSupportedError) instead, allowing callers to handle the error appropriately.
ypriverol marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Copilot
AI
Nov 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The site merging logic concatenates sites without a delimiter (e.g., "STY" for phosphorylation on S, T, and Y). However, if a site is multi-character (like "*n" for Protein N-term), this concatenation could produce ambiguous results. For example, "*n" + "K" becomes "*nK" which could be interpreted as "Protein N-term K" (a two-part site from lines 131-138) rather than two separate sites. Consider documenting this behavior or adding validation to ensure sites don't create ambiguous concatenations.
Copilot
AI
Nov 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment "merge same modification for different sites" could be more descriptive. Consider explaining the purpose and format of the merge. For example: "Merge same modification for different sites by concatenating sorted unique sites. E.g., 'Phospho,S' and 'Phospho,T' become 'Phospho,ST'". This helps developers understand the expected output format.
ypriverol marked this conversation as resolved.
Show resolved
Hide resolved
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| click | ||
| sdrf-pipelines>=0.0.31 | ||
| sdrf-pipelines==0.0.33 | ||
| pyopenms>=3.2.0 | ||
| pandas | ||
| pyarrow>=16.1.0 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -84,7 +84,20 @@ def test_diann2mztab_example(self): | |
| ] | ||
| result = run_cli_command("diann2mztab", args) | ||
| assert result.exit_code == 0 | ||
| # Additional assertions could check for expected output files | ||
|
|
||
| def test_dianncfg_example(self): | ||
| """Test generating the DIA-NN config with example data""" | ||
| args = [ | ||
| "--enzyme", | ||
| "Trypsin", | ||
| "--fix_mod", | ||
| "Carbamidomethyl (C)", | ||
| "--var_mod", | ||
| "Oxidation (M),Phospho (S),Phospho (T),Phospho (Y),Acetyl (Protein N-term),Acetyl (K),Acetyl (R),Met-loss (Protein N-term M)", | ||
| ] | ||
| result = run_cli_command("dianncfg", args) | ||
|
|
||
| assert result.exit_code == 0 | ||
|
Comment on lines
+88
to
+100
|
||
|
|
||
|
|
||
| class TestSamplesheetCommands: | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.