-
Notifications
You must be signed in to change notification settings - Fork 39
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
z-surface restraints scripts #839
Conversation
298ae56
to
13dcea8
Compare
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.
added some comment here and there. the code works! I don't know if we need to expose two commands for this..maybe only one (haddock3-restraints z_surface_restraints
) is already enough
Co-authored-by: Marco Giulini <54807167+mgiulini@users.noreply.github.com>
Co-authored-by: Marco Giulini <54807167+mgiulini@users.noreply.github.com>
Co-authored-by: Marco Giulini <54807167+mgiulini@users.noreply.github.com>
Modifications/suggestions have been addressed. |
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.
looks good! I am not sure whether to add a log call to the CLI execution
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 should be added together with the other restraints at https://github.com/haddocking/haddock-restraints, not here
I do not agree. |
There is no such thing, restraints are code agnostic and there is already a program to generate multiple types of restraint. There should be no feature duplication. This looks very trivial, if you don't have the skills to implement it I can do it |
This does not make any sense. The haddock3-restraints has been part of the code since one year. Feel free to reproduce/rewrite this wherever you like, but this feature belongs here. |
Will be a bit of duplication indeed. But the haddock3 restraints CLIs have been there since quite some time and are used in tutorials.
Of course you code is very nice Rodrigo and might supersede this implementation at some point.
But then we should make it a submodule of haddock3 which should be installed together with haddock3, e.g. in the bin dir (we could do the same for the haddock-runner).
|
Good point, we should also remove the other ones, please see: #959 @amjjbonvin why wait? Lets just do it now and avoid duplicated work and technical debt - this is a beta version :) I can also update the tutorials; @VGPReys keep this PR open as a draft for now as I see you also have the examples, we can clean it later |
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.
Considering the discussion at #959, the full replacement of the restraints interface will still require some effort since it affects the web interface. This z-surface restraints
have already been fully implemented in haddock-restraints - the duplication here is tolerable until #959 is solved
You are about to submit a new Pull Request. Before continuing make sure you read the contributing guidelines and that you comply with the following criteria:
tox
tests pass. Runtox
command inside the repository folder-test.cfg
examples execute without errors. Insideexamples/
runpython run_tests.py -b
Closes haddocking/haddock-restraints#13 by adding a new subcommand
z-surface-restraints
to the haddock3-restraints CLI.Selected residues sets (in the above example
1,2,3
and7,8,9
), will define two plans with x-dim--x-size
and y-dim--y-size
placing beads every--spacing
angstroms.Also, corresponding restraints are generated, resulting in the generation of both:
beads.pdb
file containing the coordinates of the shape beadsrestraints.tbl
file containing specific restraints to each plans.A series of examples are also provided, in
examples/docking-protein-surface
TODO LIST