-
Notifications
You must be signed in to change notification settings - Fork 1
Change format of PTM column for glycosylation #246 #249
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
base: master
Are you sure you want to change the base?
Conversation
drgbea
commented
Dec 12, 2025
- Added a unified glycosylation_site column (Header: glycosylation_site, code: 'Gly')
- Removed previous separate columns for O-GalNAc and O-GlcNAc
- Added glycosylation_subtype column, currently left empty
- Updated properties.py, datasources.py, and table.py to support the new structure
| is_gly = False | ||
| has_gly_site = False | ||
| has_gly_sub = False | ||
| for prop in position.properties.values(): | ||
| if prop.category == 'ptm_glycosylation': | ||
| is_gly = True | ||
| break | ||
|
|
||
| has_gly_site = any(prop.category == 'ptm_glycosylation' for prop in position.properties.values()) | ||
| if not has_gly_site: | ||
| gly_site = position_properties_classes['ptm_glycosylation']( | ||
| position=position, | ||
| sources=[self]) | ||
| position.add_property(gly_site) |
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.
this code assumes we will have only one glycosilation site property per position
this might be the case - do we need to also consider cases there might be different glycosilation subtypes for the same site? if e.g. our different sources have different or complementary subtypes
| is_gly = True | ||
| break | ||
|
|
||
| has_gly_site = any(prop.category == 'ptm_glycosylation' for prop in position.properties.values()) |
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.
I think this might be the source of your issue
here you use lines 1935-1941 to check which one of your properties is a Gly (ok)
however you don't handle the case in which there are no Glys. This part of the code use just prop which is whatever property comes last out of the previous for loop, whether it is a glycosilation or not (i.e. whether the break was triggered or not)
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.
mind that this section of the code is bound to be changed - read all my comments before starting to change things :)
| class GlycosylationSubtype(PositionProperty): | ||
| description = "Glycosylation Subtype" | ||
| header = "glycosylation_subtype" | ||
| category="glycosylation_subtype" | ||
| #code = |
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.
I don't think this should be a PositionProperty
this is not a feature of the protein position, it's a feature of the glycosilation itself
I think the constructor of GlycosilationSite should accept a subtype argument which would be a list of subtypes that can be at that site (as strings), so that they can be provided when creating the object and can be tracked as GlycosilationSite.subtypes