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

MMTF Parser #1752

Merged
merged 18 commits into from
Oct 4, 2023
Merged

MMTF Parser #1752

merged 18 commits into from
Oct 4, 2023

Conversation

vratins
Copy link
Contributor

@vratins vratins commented Sep 9, 2023

Added a basic parser that can read in either an mmtf file or a mmtf structure (decoder) object into an atom group. Includes header retrieval for biomolecule building.

@jamesmkrieger
Copy link
Contributor

Thanks. Can you add a test for this too, please

setup.py Outdated Show resolved Hide resolved
Added ImportError to except block for import mmtf.
Removed hard dependency for mmtf-python
@dkoes
Copy link
Collaborator

dkoes commented Sep 19, 2023

Need to add

from . import mmtffile
from .mmtffile import *
__all__.extend(mmtffile.__all__)

to proteins/__init__.py
and

__all__ = ['parseMMTF']

to mmtffile.py

dkoes and others added 5 commits September 19, 2023 15:53
Multi model still doesn't work
Support for bonds, test cases, other fixes.
At least as long is they are all the same molecule.
Support multi-model files.
@vratins
Copy link
Contributor Author

vratins commented Sep 22, 2023

Test and fixes added.

@jamesmkrieger
Copy link
Contributor

ok, it's working now. I would prefer a pip install in main.yml but I guess it's not that heavy a dependency so it's fine.

The last thing then is to give the user-facing function(s) a doc string.

Thanks again for contributing this!

@dkoes
Copy link
Collaborator

dkoes commented Oct 3, 2023

I think this is ready to be merged. Vratin still needs to add a section to the tutorial on how to use this to parse the entire PDB in a few minutes using pyspark and MMTF, but that is a different repository.

@jamesmkrieger
Copy link
Contributor

No, I think there are still a few last things to fix:

  1. The try/except for the import should be inside the function, so users don’t get the warning every time they import prody

  2. There are some redundancies between the two parts of the doc string that need to be cleaned up

Besides that, yes, it sounds good

@jamesmkrieger
Copy link
Contributor

Thanks! Very almost there now

@vratins
Copy link
Contributor Author

vratins commented Oct 3, 2023

Could you let me know what further formatting you'd like to the docstring? I'll make the changes asap.

@jamesmkrieger
Copy link
Contributor

Doc string is fine.

the last thing is for the try/except to raise ImportError

@jamesmkrieger
Copy link
Contributor

Looks good now. Thanks again!

and thanks, David, for other fixes too!

prody/proteins/mmtffile.py Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved

:arg title: Title to assign to the resulting AtomGroup. If not provided, the
title is extracted from the MMTF structure or set to the PDB ID.
:type title: str, optional
Copy link
Contributor

Choose a reason for hiding this comment

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

optional isn't really part of the type

Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally this should change but it’s ok really

prody/proteins/mmtffile.py Outdated Show resolved Hide resolved
prody/proteins/mmtffile.py Outdated Show resolved Hide resolved
@jamesmkrieger jamesmkrieger merged commit 3eb38b1 into prody:master Oct 4, 2023
5 checks passed
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.

3 participants