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.

Sign up for GitHub

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

Jump to bottom

use SI prefixes for progress #2817

Merged
merged 1 commit into from
Nov 21, 2019
Merged

use SI prefixes for progress #2817

merged 1 commit into from
Nov 21, 2019

Conversation

casperdcl
Copy link
Contributor

@casperdcl casperdcl commented Nov 19, 2019 •
edited
Loading

changes 560000/1000023 in progress bars to 560k/1M

Note that this looses some precision for large numbers but looks nicer. Not sure if this is a good or bad thing.

asciicast

@casperdcl casperdcl added enhancement Enhances DVC feature request Requesting a new feature p3-nice-to-have It should be done this or next sprint c1-quick-fix ui user interface / interaction discussion requires active participation to reach a conclusion labels Nov 19, 2019
@casperdcl casperdcl requested a review from efiop November 19, 2019 20:32
@casperdcl casperdcl self-assigned this Nov 19, 2019
@efiop
Copy link
Contributor

efiop commented Nov 20, 2019

@casperdcl Sure it is a great thing, who wants the byte precision anyway? 😄

efiop
efiop approved these changes Nov 20, 2019
View reviewed changes
Copy link
Contributor

@efiop efiop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat!

@efiop
Copy link
Contributor

efiop commented Nov 20, 2019

Is this patch all we need for all progress bars that we have? I mean, no additional modifications required for any other bars, right? Just making sure.

@efiop efiop requested review from a user and pared November 20, 2019 00:57
@casperdcl
Copy link
Contributor Author

Yes nothing else needed.

who wants the byte precision anyway?

It also reduces the file count precision when there are many files.

@efiop
Copy link
Contributor

efiop commented Nov 20, 2019

It also reduces the file count precision when there are many files.

@casperdcl Oh... How would that look now? Could you show us? Also, what are your thoughts on that from UI perspective?

@casperdcl
Copy link
Contributor Author

@efiop before/after recording added to description.

what are your thoughts on that from UI perspective?

I think in certain circumstances users may prefer granular file counts over SI prefixes.

@efiop
Copy link
Contributor

efiop commented Nov 21, 2019

Well, on the other hand, when talking about number of files, we always have complimentary progress bars for md5 computations and whatnot (and there will be additional coming) so, it is not like dvc will be looking frozen. So I think we can afford taking this behavior as a standard. Could consider a flag for granular output if someone asks for it.

@efiop efiop merged commit 5c5d1ed into iterative:master Nov 21, 2019
@casperdcl casperdcl deleted the si-progress branch November 21, 2019 23:52
@weekly-digest weekly-digest bot mentioned this pull request Nov 24, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@efiop efiop efiop approved these changes

@pared pared pared approved these changes

Assignees

@casperdcl casperdcl

Labels
discussion requires active participation to reach a conclusion enhancement Enhances DVC feature request Requesting a new feature p3-nice-to-have It should be done this or next sprint ui user interface / interaction
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@casperdcl @efiop @pared