-
Notifications
You must be signed in to change notification settings - Fork 33
[1pt] PR: Minor adj to test_case_by_hydroid #1584
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
base: dev
Are you sure you want to change the base?
Conversation
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.
Looks good. Just a comment that the progress bar can be disabled in tqdm()
but removing it might be a better solution here.
|
||
missing_hucs = [] | ||
|
||
# easier to filter by one huc for debugging purposes | ||
# debug_test_hucs = ["12090301", "07100007", "19020302"] | ||
# tqdm will likely not work with all of the printd | ||
for test_case_class in tqdm(all_test_cases, desc=f'Running {len(all_test_cases)} test cases'): |
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.
tqdm()
has an optional argument disable=True
that will prevent the progress bar from being displayed. But I'm not sure why to keep using tqdm
in that case unless you wanted to be able to set disable
dynamically (e.g., command-line vs. batch).
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.
Yes. I knew we could remove it but it will no longer have value, so we are removing it. Generally all of our products will eventually remove tqdm progress bars for the same reason. If you want print statements in a piece of code inside a block with t2dm, it just paints the progress bar with its current status (ie.. 20 of 200), then prints something on the next line(s), then tqdm needs to full repaint the bar to show 21 of 200). It can not update the progress bar, print a value, then come back up to adjust the tqdm. In lieu, I believe it is best just to print lines as "processing 20 of 200"... print, print, "processing 21 of 200".. The point of having these various types of process includes "how far along are we" and "is it still running if we don't see any prints or something for a while".
Just a few minor tweaks:
Changes
tools\test_case_by_hydroid.py
: as describedTesting
Ran it at full scale and validated the outputs for the number of recs inside the FIM Performance catchment output file.
Deployment Plan (For developer use)
How does the changes affect the product?
Issuer Checklist (For developer use)
You may update this checklist before and/or after creating the PR. If you're unsure about any of them, please ask, we're here to help! These items are what we are going to look for before merging your code.
[_pt] PR: <description>
dev
branch (the default branch), you have a descriptive Feature Branch name using the format:dev-<description-of-change>
(e.g.dev-revise-levee-masking
)dev
branchpre-commit
hooks were run locally4.x.x.x
Merge Checklist (For Technical Lead use only)