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

foldx Buildmodel mutation interfact - convenience interface #374

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Mojusko
Copy link

@Mojusko Mojusko commented Feb 23, 2022

Not sure this is interesting to you.

@padix-key
Copy link
Member

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?

@Mojusko
Copy link
Author

Mojusko commented Feb 24, 2022

  1. It introduces a mutation to a protein a generates a structure for it. Its part of larger suite which prepares mutated proteins for MD simulations 2. Indeed its proprietary, but so is vina which also has an interface. I would like to know open-source version of this as well. A similar functionality is contained in proprietary version of pymol. I was not sure if this is something interesting to this community. Anyway I like your package. Let me know if I should finish this.

@padix-key
Copy link
Member

Basically every interface for publicly available and well documented software that works on sequence or structure data is interesting for biotite.application. I will have a look on your PR and prepare a review.

Regarding your questions on open-source versions of AutoDock Vina and PyMOL:
To my knowledge Vina is exclusively open source (https://vina.scripps.edu/license/), while for PyMOL only a basic version (without officially built binaries) is open-source (https://github.com/schrodinger/pymol-open-source/blob/master/LICENSE).

Copy link
Member

@padix-key padix-key left a 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:

  1. 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.

  2. 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"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
__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"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
__name__ = "biotite.application.foldX"
__name__ = "biotite.application.foldx"

Comment on lines +9 to +21
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
Copy link
Member

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):
Copy link
Member

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:

Suggested change
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.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The structure of the proiten molecule.
The structure of the protein molecule.

The mutated protein

"""
return self.new_mutant
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return self.new_mutant
return self._new_mutant

Comment on lines +39 to +40
file = PDBFile()
assert (file is not None)
Copy link
Member

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()
Copy link
Member

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
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
import biotite.structure.info as info

# information.

from os.path import join
import numpy as np
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
import numpy as np

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