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: pbar during md5 calc doesn't need to be multithreaded #2539

Closed
Suor opened this issue Sep 27, 2019 · 7 comments · Fixed by #2543
Closed

ui: pbar during md5 calc doesn't need to be multithreaded #2539

Suor opened this issue Sep 27, 2019 · 7 comments · Fixed by #2543
Assignees
Labels
enhancement Enhances DVC p2-medium Medium priority, should be done, but less important ui user interface / interaction

Comments

@Suor
Copy link
Contributor

Suor commented Sep 27, 2019

The "Mutithreaded" prefix is confusing, also jumping through screen doesn't work with redirection well.

@efiop efiop added ui user interface / interaction enhancement Enhances DVC p2-medium Medium priority, should be done, but less important labels Sep 27, 2019
@shcheklein
Copy link
Member

💯 on this!

@casperdcl
Copy link
Contributor

@Suor just to confirm for other multi-threaded commands is screen still giving you trouble?

perhaps this is tqdm/tqdm#798 - you may need screen -U

casperdcl added a commit to casperdcl/dvc that referenced this issue Sep 28, 2019
@Suor
Copy link
Contributor Author

Suor commented Sep 29, 2019

Never ran dvc in screen. I had, however, similar problem when redirecting dvc output, special line up sequence got swallowed somewhere in the process.

@efiop
Copy link
Contributor

efiop commented Sep 29, 2019

Btw, @casperdcl , just wondering, does tqdm use is_tty to avoid printing some things when it is not using a console?

@casperdcl
Copy link
Contributor

@efiop no, that would be an interesting feature request. The thing is if you pipe output to a file and then cat the file, often the control characters are parsed properly.

@efiop
Copy link
Contributor

efiop commented Sep 29, 2019

@casperdcl Yeah, but you usually don't cat if you redirect it to a file :) If you are redirecting, then you want a log file to read later(to grep, to vim or whatever), which doesn't need control chars and they might even be harmful. I'm pretty sure I've seen tools use isatty to distinguish those cases a lot. Our old progress bar did that too if I recall correctly. The downside of that approach is that now you suddenly have two types of outputs, which might cause some things to not get enough testing. So there are pros and cons to both approaches, for sure.

@casperdcl
Copy link
Contributor

casperdcl commented Sep 29, 2019

nvm correction this was solved way back in tqdm/tqdm#281 - it's just not propagated to dvc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhances DVC p2-medium Medium priority, should be done, but less important ui user interface / interaction
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants