-
-
Notifications
You must be signed in to change notification settings - Fork 132
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
control protein graph construction verbosity #242
Conversation
@a-r-j Seems the docker build is failing. I didn't edit the build at all, is it working on master? |
It can be a little unstable. I'm quite confident a rebuild will resolve the problem. |
Codecov ReportBase: 40.27% // Head: 47.93% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #242 +/- ##
==========================================
+ Coverage 40.27% 47.93% +7.66%
==========================================
Files 48 92 +44
Lines 2811 5526 +2715
==========================================
+ Hits 1132 2649 +1517
- Misses 1679 2877 +1198
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
graphein/protein/graphs.py
Outdated
@@ -697,12 +701,13 @@ def construct_graph( | |||
if config is None: | |||
config = ProteinGraphConfig() | |||
with Progress(transient=True) as progress: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think it would be better to use a pattern like this:
from contextlib import nullcontext
cm = Progress(transient=True) if verbose else nullcontext():
with cm as progress:
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yeah thats really nice, didn't know about nullcontext. I'll refactor
Kudos, SonarCloud Quality Gate passed!
|
Reference Issues/PRs
What does this implement/fix? Explain your changes
Fixes #238
Adds verbosity control to the protein graph construction. The drawback is that this solution still involves the creation of a Progress context even if no progress is shown. However, this overhead is minimal and obviates the need for try/catch/finally if using Progress non-contextually or shallow wrappers in construct_graph.
What testing did you do to verify the changes in this PR?
Passes the included tests.
Pull Request Checklist
./CHANGELOG.md
file (if applicable)./graphein/tests/*
directories (if applicable)./notebooks/
(if applicable)python -m py.test tests/
and make sure that all unit tests pass (for small modifications, it might be sufficient to only run the specific test file, e.g.,python -m py.test tests/protein/test_graphs.py
)black .
andisort .