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

Graph representation of biological model #131

Merged
merged 19 commits into from
Jun 23, 2022
Merged

Graph representation of biological model #131

merged 19 commits into from
Jun 23, 2022

Conversation

formersbach
Copy link
Contributor

Will add support for graphic representation of biological model.
Requires graphviz program and pygraphviz package.
Minor changes to exec_model.py to make create_graph callable on model object.

@himoto himoto self-requested a review June 23, 2022 00:43
@lgtm-com
Copy link
Contributor

lgtm-com bot commented Jun 23, 2022

This pull request introduces 1 alert when merging 92640c6 into f7d3484 - view on LGTM.com

new alerts:

  • 1 for Except block handles 'BaseException'

@lgtm-com
Copy link
Contributor

lgtm-com bot commented Jun 23, 2022

This pull request introduces 1 alert when merging cda1212 into f7d3484 - view on LGTM.com

new alerts:

  • 1 for Unused import

@lgtm-com
Copy link
Contributor

lgtm-com bot commented Jun 23, 2022

This pull request introduces 1 alert when merging bce630c into f7d3484 - view on LGTM.com

new alerts:

  • 1 for Unused import

Copy link
Contributor

@himoto himoto left a comment

Choose a reason for hiding this comment

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

Hi @formersbach, Thanks a million for implementing a graph visualization method!
Overall, this update is amazing. The code needs minor update before merging into master.

biomass/graph.py Outdated
import os
import re
import warnings
from ast import Is
Copy link
Contributor

Choose a reason for hiding this comment

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

@formersbach from ast import Is Can you please remove this unused import?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Weirdly this import was not added by me (at least not intentionally). I'll remove it, but do you have any idea where it might've come from?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it is from VScode auto-import when you type "Is". I sometimes face the same problem.

biomass/graph.py Outdated
Comment on lines 128 to 135
def to_graph(
self,
save_path: str,
gviz_prog: str = "dot",
):
"""
Constructs and draws a directed graph of the model using ode.py/reaction_network.py as text files.
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

@formersbach It would be better to add Parameters and Examples section to docstring. I would like you to follow numpydoc style.

@himoto
Copy link
Contributor

himoto commented Jun 23, 2022

Thanks @formersbach! I have updated docs and will soon merge your work into master.

@himoto himoto merged commit dfb0d67 into master Jun 23, 2022
@himoto himoto deleted the graphviz branch June 23, 2022 08:52
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