-
Notifications
You must be signed in to change notification settings - Fork 37
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
Added the no-progress flag for project.py status to hide the progress… #685
Conversation
Codecov Report
@@ Coverage Diff @@
## main #685 +/- ##
==========================================
+ Coverage 69.10% 69.28% +0.17%
==========================================
Files 43 43
Lines 4279 4294 +15
Branches 1044 1047 +3
==========================================
+ Hits 2957 2975 +18
+ Misses 1109 1108 -1
+ Partials 213 211 -2
|
Co-authored-by: Brandon Butler <[email protected]>
c2f2d49
to
9225393
Compare
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.
Thanks for working on this @iblanco11981870! I had to rebase this PR because it contained some merged commits that were included by accident and made the PR diff appear incorrectly. Please run these commands to fix your local branch:
$ git fetch --all
$ git checkout feature/print_status
$ git reset --hard origin/feature/print_status
I have some design input as well. See comments. Thank you!
Co-authored-by: Bradley Dice <[email protected]>
Co-authored-by: Bradley Dice <[email protected]>
…signac-flow into feature/print_status
What more needs to be done to complete this pull request? I would like to use it to clean up the output in the HOOMD-blue tutorials. |
@joaander I think we're just waiting on a reviewer. I addressed all the suggested changes. |
OK, the reviewers may not know unless you click the "re-request review" button or otherwise communicate that to them. |
@iblanco11981870 Can you please resolve conflicts on this branch before I review? Thanks! |
for more information, see https://pre-commit.ci
We need to use cloudpickle to serialize/deserialize functions for process parallelization.
Converts the generator to a list which is what print_status expects.
flow/project.py
Outdated
progress : bool | ||
Show a progress bar during execution. | ||
hide_progress : bool | ||
Hides progress bar when printing status output (Default value = True). |
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.
There is no default value set here.
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.
Has been reverted.
flow/project.py
Outdated
"--hide_progress", | ||
action="store_false", |
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.
Trying to make sure I understand here -- why is this store_false
and the other line above is store_true
? Does the progress behavior match the behavior prior to this PR?
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.
This did change behavior and has been removed.
@iblanco11981870 Can we push this through or close it? |
ab37ab0
to
8f3cead
Compare
8f3cead
to
02cbebf
Compare
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.
@b-butler Thanks for requesting my review on this. I took one brief pass but can't dedicate more time to this PR. Please merge when you think it is sufficiently ready.
Co-authored-by: Bradley Dice <[email protected]>
Still results in shorter clearer code.
Thanks! |
Description
Added the no-progress flag for
FlowProject.print_status
. This hides the progress bar output generated whenFlowProject.print_status
, and adds the options to hide the progress bar when desired.Motivation and Context
The progress bar displays a significant amount of the output for small workspaces and does not give any benefit. Additionally, using
FlowProject.print_status
in Jupyter notebooks requires special configurations or else it does not work. This addresses issue #602Checklist: