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

Raise an issue if a link is not applied correctly #325

Closed
wants to merge 1 commit into from

Conversation

Romumrn
Copy link

@Romumrn Romumrn commented May 30, 2023

I noticed a problem creating a peptide chain with a link between 2 non-consecutive amino acids.
I was not able to fix this issue but I think that the bug is in apply_links.py with _check_relative_order.
So I just raised an issue in the log, if all the links are not applied correctly. It would be very useful for MAD and also for polyply users.

@fgrunewald
Copy link
Member

@Romumrn I don't quite understand the issue you describe. If I try to make a circular poly valine 8mer and connect the 1st and 5th residue I get the linear parameters, which is in fact what I would expect as you cannot connect two valines together via their side chain.

Perhaps you mean that we should warn the user in this case, because they their input protein is circular but they get the linear parameters out? The linear parameters are valid and correct, but not the one that is expected. So in some sense that can be confusing. I agreee. However, if this is the issue you mean, we need to make sure that moving the check doesn't mess with anything else especially the protein modification. Also it is very expensive for long polymers but that we can fix.

Anyways, let me know if I understand the issue correctly.

Comment on lines +105 to +109
#Check missing edges and raise issues if links between amino acids are not applied.
#Bug if the backbone is correct links between amino acids in a chain are not applied correctly
for missing in find_missing_edges(meta_molecule, meta_molecule.molecule):
msg = "Link between residue {idxA} {resA} and residue {idxB} {resB} was not applied."
LOGGER.warning(msg, **missing)
Copy link
Member

Choose a reason for hiding this comment

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

Instead of just adding this line we should check if there are missing edges and then raise a general error message like the "disconnected molecule warning" and then proceed to list the edges. The connected molecule check becomes then redundant and can be removed.

@Romumrn
Copy link
Author

Romumrn commented Jun 15, 2023

Hi Fabian,

Sorry for the delay in my response. Yes, it is indeed not possible to establish a circular peptide. However, when a MAD user wants to do it, I don't have any feedback about linking it. So I wanted to use it to warn the user through an error as you did before for the link warning that has no connection rules.

I find this warning to be extremely helpful, as it allows me to provide feedback to the user. Do you think it would be doable to maintain it, or would you suggest a more global approach?

@fgrunewald
Copy link
Member

@Romumrn I think we can do it, but I suggest we introduce a CLI flag. Something like polyply gen_params -strict, which will then apply the check. Otherwise it might reduce the performance for large molecules such as the genome quite drastically.

Could you send me some example .json files by mail, which you think should raise an error?

@fgrunewald
Copy link
Member

this will be included in PR #313

@fgrunewald fgrunewald closed this Aug 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants