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

ui: remove multi threaded prefix before each progress bar #2686

Closed
shcheklein opened this issue Oct 29, 2019 · 10 comments · Fixed by #2703
Closed

ui: remove multi threaded prefix before each progress bar #2686

shcheklein opened this issue Oct 29, 2019 · 10 comments · Fixed by #2703
Assignees
Labels
enhancement Enhances DVC good first issue hacktoberfest p1-important Important, aka current backlog of things to do research ui user interface / interaction

Comments

@shcheklein
Copy link
Member

When you run even a simple dvc pull for a single dvc file for a project that has only one data file it still shows the Multi threaded prefix before the progress bar.

git clone https://github.com/iterative/example-get-started
cd example-get-started
dvc pull

outputs some like:

Multi-Threaded:
 17%|█▋        |data/prepared/train.ts...

Commands like dvc pull, dvc push, etc are affected. I would expect to see something meaningful instead: Downloading files (multiple threads) or even w/o multiple threads.

Sometimes this Multi-Threaded: is blinking and replacing the progress bar itself which makes experience even worse.

@shcheklein shcheklein added enhancement Enhances DVC good first issue hacktoberfest ui user interface / interaction p1-important Important, aka current backlog of things to do labels Oct 29, 2019
@harsh8398
Copy link
Contributor

@shcheklein you just want different static message right? or want some conditional message?

@kurianbenoy
Copy link
Contributor

I would like to work on this, I am curious what is causing this Multi-Threaded message to come though?

@shcheklein
Copy link
Member Author

@harsh8398 it's better to have a conditional message - that describes what is happening depending on the command.

We def should remove the blinking behavior.

@kurianbenoy feel free! but please coordinate with @harsh8398 if needed and please ask an advice with @casperdcl - an expert in TQDM :)

@casperdcl
Copy link
Contributor

casperdcl commented Oct 29, 2019

blank_bar = Tqdm(bar_format="Multi-Threaded:", leave=False)

but @shcheklein

Sometimes this Multi-Threaded: is blinking and replacing the progress bar itself which makes experience even worse.

shouldn't happen - is this still a thing after #2546?

@shcheklein
Copy link
Member Author

@casperdcl I think it is still happening (on dvc pull). But I'll check again on the latest dev version.

@casperdcl
Copy link
Contributor

I'm referring to specifically the blinking and overwriting. Shouldn't happen.

Anyway there has to at least be a temporary blank line (could replace "Multi-Threaded:" with an empty string) as that's by far the most painless way of letting tqdm know about being in multi-threaded mode.

@Suor
Copy link
Contributor

Suor commented Oct 30, 2019

Didn't get the last thing. Why tqdm can't be in multi-threaded mode without stating that with a message?

@casperdcl
Copy link
Contributor

casperdcl commented Oct 30, 2019

dvc/dvc/progress.py

Lines 20 to 21 in 877e385

Creates a blank initial dummy progress bar if needed so that workers
are forced to create "nested" bars.

I think the main idea is https://github.com/tqdm/tqdm/blob/91494ccf363e5eeb4745db7dbab70ef73c4a6eb0/tqdm/std.py#L1240 but due to

leave=False,
it might not be an issue any more.

Ah wait yes of course - it we assume all threads have leave=False then it's safe to remove the message. Otherwise they may wind up printing out of order.

@kurianbenoy
Copy link
Contributor

So @casperdcl will it be possible to write conditional messages for each command as @shcheklein said?

@casperdcl
Copy link
Contributor

casperdcl commented Oct 30, 2019

I think missing conditional messages/verbosity is unrelated to the "remove unhelpful multi-threading message" issue. The first is a long term goal involving carefully relooking at each commamnd. The latter could probably be fixed by just removing the fake bar altogether...

efiop added a commit that referenced this issue Nov 2, 2019
Remove multi-threaded prefix before progress bar(#2686)
@casperdcl casperdcl self-assigned this Nov 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhances DVC good first issue hacktoberfest p1-important Important, aka current backlog of things to do research ui user interface / interaction
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants