-
Notifications
You must be signed in to change notification settings - Fork 94
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
foldx Buildmodel mutation interfact - convenience interface #374
base: main
Are you sure you want to change the base?
Conversation
Hi, thank you for adding the interface. Initially, I have two requests: 1. Could you give a short description in the PR, what the interfaced software does? 2. Via a quick search I found out that this is a proprietary program. This is not a problem in general, but renders automatic testing in the CI pipeline difficult. Is there an open source version of the software? |
|
Basically every interface for publicly available and well documented software that works on sequence or structure data is interesting for Regarding your questions on open-source versions of AutoDock Vina and PyMOL: |
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.
Hi, I reviewed your FoldX interface and I think it is written well in general. However, I have two more major requests:
-
I think the docstrings are a bit sparse, especially regarding the purpose of the application and the description of the parameters. Since in my opinion the functionalities of the software are rather complex, I think a few more words would be helpful.
-
The
rotabase.txt
is packed into Biotite. While this is convenient to the user, I assume this is not allowed by the commercial FoldX license. If I am correct, this unfortunately means that the rotabase needs to be provided as additional input by the user.
I need to emphasize, that I am new to the FoldX software. Hence some of my comments may be incorrect.
A subpackage for static ligand docking with *FoldX*. | ||
""" | ||
|
||
__name__ = "biotite.application.foldX" |
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.
__name__ = "biotite.application.foldX" | |
__name__ = "biotite.application.foldx" |
# under the 3-Clause BSD License. Please see 'LICENSE.rst' for further | ||
# information. | ||
|
||
__name__ = "biotite.application.foldX" |
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.
__name__ = "biotite.application.foldX" | |
__name__ = "biotite.application.foldx" |
import copy | ||
from tempfile import NamedTemporaryFile | ||
import numpy as np | ||
import os | ||
from os import chdir, getcwd, remove | ||
from ..localapp import LocalApp, cleanup_tempfile | ||
from ..application import AppState, requires_state | ||
from ...structure.io.pdbqt import PDBQTFile | ||
from ...structure.io.pdb import PDBFile | ||
from ...structure.io.pdbx import PDBxFile | ||
from ...structure.residues import get_residue_starts_for, get_residue_masks | ||
from ...structure.bonds import find_connected | ||
from ...structure.error import BadStructureError |
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.
A lot of these imports are actually unused. Please remove the unused imports, when you finished your work on the PR.
from ...structure.error import BadStructureError | ||
|
||
|
||
class FoldXApp(LocalApp): |
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.
As I read so far, FoldX has multiple subcommands, and the one used here is called BuildModel
. Hence, i propose the following:
class FoldXApp(LocalApp): | |
class BuildModelApp(LocalApp): |
This is consistent with e.g. the viennarna
subpackage, where the classes are called RNAfoldApp
and RNAplotApp
.
Parameters | ||
---------- | ||
receptor : AtomArray, shape=(n,) | ||
The structure of the proiten molecule. |
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.
The structure of the proiten molecule. | |
The structure of the protein molecule. |
The mutated protein | ||
|
||
""" | ||
return self.new_mutant |
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.
return self.new_mutant | |
return self._new_mutant |
file = PDBFile() | ||
assert (file is not None) |
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 line is an error: A newly created PDBFile
is never None
.
app = FoldXApp(receptor, mutation, subunit = 'B') | ||
app.start() | ||
app.join() | ||
arr = app.get_mutant() |
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.
An actual check of the mutated output structure would be good here. Even if the actually output atom positions cannot be easily tested, I think it would be sufficient to check if the original atoms are still there, except the mutation position
import numpy as np | ||
import pytest | ||
import biotite.structure as struc | ||
import biotite.structure.info as info |
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.
import biotite.structure.info as info |
# information. | ||
|
||
from os.path import join | ||
import numpy as np |
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.
import numpy as np |
Not sure this is interesting to you.