Skip to content
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

Fix double modications #188

Merged
merged 20 commits into from
Jul 17, 2024
Merged

Fix double modications #188

merged 20 commits into from
Jul 17, 2024

Conversation

mschwoer
Copy link
Contributor

@mschwoer mschwoer commented Jun 25, 2024

Fix the issues discovered here #186
by removing the double-bookeeping in the modifications.

@jalew188 and me decided to break the public API (the methods in modifications.py) but not the reading of speclib files
This is how is looks like in alphaDIA:
image

Please check very carefully this breaks something you know about

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link
Collaborator

@jalew188 jalew188 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

I saw unimod_to_tsv.ipynb was excuted, did you overwrite modification.tsv?

@mschwoer
Copy link
Contributor Author

LGTM.

I saw unimod_to_tsv.ipynb was excuted, did you overwrite modification.tsv?

yes I did

Copy link
Collaborator

@GeorgWa GeorgWa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, this was long necessary.
Will be breaking with the default yaml.

@mschwoer mschwoer marked this pull request as ready for review July 12, 2024 11:57
@jalew188
Copy link
Collaborator

@mschwoer @GeorgWa ready to merge!!

@mschwoer mschwoer merged commit bb624c0 into development Jul 17, 2024
2 checks passed
@mschwoer mschwoer deleted the fix_double_modications branch July 17, 2024 09:34
'Acetyl@Protein N-term':
- '_(Acetyl (Protein N-term))'
'Acetyl@Protein_N-term':
- '_(Acetyl (Protein_N-term))'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The actual unimod names contain a space, so we cannot modify the MQ source PTM names

Copy link
Contributor Author

@mschwoer mschwoer Nov 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

released in 1.3.0, fixed here for 1.4.2: #241

@mschwoer mschwoer mentioned this pull request Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants