fixed bugs #234
Merged
fixed bugs #234
Conversation
This file contains 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
Update DIANNCONVERT
Fix/conf warnings
This was
linked to
issues
Oct 20, 2022
|
jpfeuffer
reviewed
Oct 20, 2022
jpfeuffer
reviewed
Oct 20, 2022
| @@ -108,6 +110,24 @@ workflow DIA { | |||
|
|
|||
| } | |||
|
|
|||
|
|
|||
| // remove meta.id to make sure cache identical HashCode | |||
There was a problem hiding this comment.
Can you explain why we cannot set an ID that would be unique instead (of removing the ID completely)?
There was a problem hiding this comment.
The id of each meta is different. But we only get the parameter values once. When unique or first is used, the first run may return meta=[id:1,.. ] but when run again with resume, it may return meta=[id:2,...], possibly due to parallel mechanism. So there is no cache success and it is recognized by nextflow as a new run.
I don't know if I explained it clearly 😂
There was a problem hiding this comment.
Where is meta.id set? Do we have control? Then we can just set it to a unique value per experiment.
There was a problem hiding this comment.
quantms/subworkflows/local/create_input_channel.nf
Lines 76 to 78 in 1ace2fb
There was a problem hiding this comment.
But why does it change then??
There was a problem hiding this comment.
But we still need to remove id for DIA, even if the experience id is added. Because different meta channels may be generated at runtime, resulting in cache failure.
There was a problem hiding this comment.
When I first run, the channel emits meta [id: 1,], but when re-run again, the channel maybe emit meta [id: 2,]. Because they are parallel
There was a problem hiding this comment.
Yes I think meta should look like this:
[
{mzml_ref: foo, exp_ref: designname, param_a: xyz, param_b:xyz }
{mzml_ref: bar, exp_ref: designname, param_a: xyz, param_b:xyz }
]
There was a problem hiding this comment.
Or we just use param.input as tag for those steps.
There was a problem hiding this comment.
Maybe we can rename id to mzml_id or mzml_ref, to make it clear what it is for
ypriverol
approved these changes
Oct 21, 2022
ypriverol
requested changes
Oct 21, 2022
bin/diann_convert.py
Outdated
| :rtype: float or NoneType | ||
| """ | ||
| if "X" in seq: | ||
| seq = seq.replace("X", "") |
There was a problem hiding this comment.
You have here multiple sequence possibilities, from the top of my head I remember X, B, Z. I think it is better to create a white list including the 22 amino acids. (ARNDBCEQZGHILKMFPSTWYV).
There was a problem hiding this comment.
The question is also: Is this really completely unknown? Or does it have a mass associated with it as modification for example. Because just counting 0 for an unknown amino acid might be a bit too simple.
There was a problem hiding this comment.
In uniprot fasta you don't have anything associated to it. Then we have two options, or we put null in the value, or we assume 0 (which is not ideal but is the only way to compute this in mass spec terms).
There was a problem hiding this comment.
But how can DIANN identify it then?
There was a problem hiding this comment.
I have no idea. @vdemichev How DIANN handles the unknown sequences in Uniprot, for example peptides with X. I mean to compute the theoretical mass for example.
There was a problem hiding this comment.
X is interpreted as selenomethionine, an unknown symbol like '*' would be interpreted as either zero mass or glycine, not sure. In general, behaviour on nonsense sequences is undefined.
There was a problem hiding this comment.
@daichengxin can you change this accordingly.
jpfeuffer
approved these changes
Oct 24, 2022
ypriverol
approved these changes
Oct 24, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
PR checklist
nf-core lint).nextflow run . -profile test,docker --outdir <OUTDIR>).docs/usage.mdis updated.docs/output.mdis updated.CHANGELOG.mdis updated.README.mdis updated (including new tool citations and authors/contributors).