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.

Sign up for GitHub

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

Jump to bottom

refactor: unify buildgraph.py and build_graph functions for atom and residue level graphs #507

Merged
merged 6 commits into from
Nov 23, 2023
Merged

refactor: unify buildgraph.py and build_graph functions for atom and residue level graphs #507

merged 6 commits into from
Nov 23, 2023

Conversation

DaniBodor
Copy link
Collaborator

@DaniBodor DaniBodor commented Sep 23, 2023 •
edited
Loading

This PR is easiest reviewed one commit at a time.

These are the changes, in order of commits:

  • creates central amino acid dictionaries instead of having them created multiple times at different parts throughout the code.
  • improve formatting (line spacing, etc) and documentation (fewer comments, more docstrings, etc.) of buildgraph.py
  • simplified code that reads atom data from pdb2sql objects
  • simplified code that finds residues within influence_radius
  • unify functions for building atomic and residue graphs (last 2 commits)

fix: #506, #503

blocked by: #492, #504

@DaniBodor DaniBodor linked an issue Sep 23, 2023 that may be closed by this pull request
create amino acid dictionary in deeprank2/domain/aminoacidlist.py #503
Closed
@DaniBodor DaniBodor changed the base branch from main to 460_radius_vs_edgedistance_dbodor September 23, 2023 14:57
@DaniBodor DaniBodor force-pushed the 460_radius_vs_edgedistance_dbodor branch from d2bebeb to 702f1ed Compare September 23, 2023 15:16
@DaniBodor DaniBodor force-pushed the 506_buildgraph_unification_dbodor branch 2 times, most recently from 8060040 to f7d2c9a Compare September 23, 2023 18:05
@DaniBodor DaniBodor force-pushed the 460_radius_vs_edgedistance_dbodor branch from 1ba344d to 8c710bc Compare September 23, 2023 18:07
@DaniBodor DaniBodor force-pushed the 506_buildgraph_unification_dbodor branch 2 times, most recently from 131ea78 to d370a90 Compare September 23, 2023 18:14
@DaniBodor DaniBodor marked this pull request as draft September 23, 2023 18:14
@DaniBodor DaniBodor force-pushed the 460_radius_vs_edgedistance_dbodor branch from 8c710bc to a1ccfc1 Compare September 23, 2023 18:23
@DaniBodor DaniBodor force-pushed the 506_buildgraph_unification_dbodor branch 4 times, most recently from 53ff56b to 7bf653d Compare September 23, 2023 21:26
@github-actions
Copy link

github-actions bot commented Oct 9, 2023

This PR is stale because it has been open for 14 days with no activity.

@github-actions github-actions bot added the stale issue not touched from too much time label Oct 9, 2023
@DaniBodor DaniBodor force-pushed the 460_radius_vs_edgedistance_dbodor branch 2 times, most recently from 29edf9a to 27e3746 Compare October 10, 2023 07:54
@DaniBodor DaniBodor force-pushed the 506_buildgraph_unification_dbodor branch from 7bf653d to 4bab1d7 Compare October 10, 2023 08:03
@DaniBodor DaniBodor force-pushed the 460_radius_vs_edgedistance_dbodor branch from 27e3746 to 3e81057 Compare November 3, 2023 16:09
@DaniBodor DaniBodor force-pushed the 506_buildgraph_unification_dbodor branch from 4bab1d7 to 40c9c60 Compare November 3, 2023 16:11
@DaniBodor DaniBodor force-pushed the 460_radius_vs_edgedistance_dbodor branch from 3e81057 to 2e8dc67 Compare November 3, 2023 16:26
@DaniBodor DaniBodor force-pushed the 506_buildgraph_unification_dbodor branch from 40c9c60 to 982b8c0 Compare November 3, 2023 16:27
@DaniBodor DaniBodor force-pushed the 460_radius_vs_edgedistance_dbodor branch from 2e8dc67 to 6df8f63 Compare November 3, 2023 16:54
@DaniBodor DaniBodor force-pushed the 506_buildgraph_unification_dbodor branch 3 times, most recently from 4de1652 to 5f29c08 Compare November 3, 2023 17:29
@DaniBodor DaniBodor removed the stale issue not touched from too much time label Nov 7, 2023
@gcroci2 gcroci2 mentioned this pull request Nov 8, 2023
Closed
Base automatically changed from 460_radius_vs_edgedistance_dbodor to dev November 17, 2023 22:58
@DaniBodor DaniBodor force-pushed the 506_buildgraph_unification_dbodor branch from 5f29c08 to 8b9bd3a Compare November 17, 2023 23:16
@DaniBodor DaniBodor marked this pull request as ready for review November 17, 2023 23:19
@DaniBodor DaniBodor changed the title refactor buildgraph.py and build_graph functions refactor: unify buildgraph.py and build_graph functions for atom and residue level graphs Nov 18, 2023
@DaniBodor DaniBodor force-pushed the 506_buildgraph_unification_dbodor branch from 8b9bd3a to a5fb3e9 Compare November 18, 2023 11:13
@DaniBodor DaniBodor requested a review from gcroci2 November 18, 2023 11:32
@DaniBodor DaniBodor force-pushed the 506_buildgraph_unification_dbodor branch from a5fb3e9 to 03e74b3 Compare November 18, 2023 11:34
gcroci2
gcroci2 approved these changes Nov 22, 2023
View reviewed changes
Copy link
Collaborator

@gcroci2 gcroci2 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Very cleaner and useful edits :D I left a few very minor comments/suggestions, then feel free to merge

tests/domain/test_aminoacidlist.py Outdated Show resolved Hide resolved
deeprank2/utils/graph.py Outdated Show resolved Hide resolved
@DaniBodor
refactor: amino acid dictionaries
2b69803 
create dictionaties of amino acids by letter, 3-letter code, and name
and use these in other modules rather than defining similar structures locally
@DaniBodor
docs: improve code documentation of utils/buildgraph.py
0d994de 
type hinting, docstrings, code comments, excessive white lines, etc
@DaniBodor DaniBodor force-pushed the 506_buildgraph_unification_dbodor branch from feb25bd to 9103e45 Compare November 22, 2023 20:46
@DaniBodor
refactor: reading atom data from pdb2sql object
60775bd
@DaniBodor
refactor: finding residue from pdb2sql key
da0f510 
by using a helper function rather than repeating code
@DaniBodor
style: make build_atomic_graph and build_residue_graph look similar
2708c5c 
this is a preparation for the next commit, where the functions are unified.
here only variables are renamed and spacing is changed so that it is easy to see what
the similarities and differences between the functions are.
@DaniBodor
refactor: unify build_graph functions
a777cb7 
previously separate functions for building atom and residue graphs
@DaniBodor DaniBodor force-pushed the 506_buildgraph_unification_dbodor branch from 9103e45 to a777cb7 Compare November 22, 2023 20:54
@DaniBodor DaniBodor merged commit 9d508c0 into dev Nov 23, 2023
6 checks passed
@DaniBodor DaniBodor deleted the 506_buildgraph_unification_dbodor branch November 23, 2023 09:50
This was referenced Dec 1, 2023
Closed
Closed
@gcroci2 gcroci2 added the SS label Jan 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gcroci2 gcroci2 gcroci2 approved these changes

Assignees
No one assigned
Labels
SS
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Refactor buildgraph module create amino acid dictionary in deeprank2/domain/aminoacidlist.py
2 participants
@DaniBodor @gcroci2