search

a-r-j / graphein

Protein Graph Library
https://graphein.ai/
MIT License
1.03k stars 131 forks source link

Process dataframe docstrings #27

Closed ericmjl closed 3 years ago

ericmjl commented 3 years ago

@a-r-j requesting a short review on this one.

I added docstrings to the process_dataframe function, and slightly modified its implementation for simplicity.

One thing I'm a bit unsure of: if granularity == something shows up in two places. Do you think this can be simplified? "granularity" as a category of properties to worry about -- if it can be handled in one block of code rather than two, that would help with reasoning about the function and debugging/maintenance later on.

a-r-j commented 3 years ago

LGTM, thanks Eric!

I think you're right about granularity - it's doing too much. The idea is to have control over:

  1. What the nodes are in a residue graph (e.g. a-Carbon "CA", b-Carbon "CB")
  2. Whether to use that atom position or compute residue centroids
  3. Whether or not to build a residue-graph as above or an atom-graph
ericmjl commented 3 years ago

@a-r-j ok! That's definitely for another PR. Gonna raise an issue so that we can track this. :smile: