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

ls/ls-url: use humanize.naturalsize instead of tqdm #9857

Merged
merged 1 commit into from
Aug 17, 2023

Conversation

efiop
Copy link
Contributor

@efiop efiop commented Aug 17, 2023

Currently we get odd formatting like

456    .devcontainer.json
139    .dvcignore
61.0   .gitattributes
96.0   .github
7.00   .gitignore
5.76k  README.md
160    data
1.84k  dvc.lock
519    dvc.yaml
4.00   foo
96.0   models
96.0   notebooks
225    params.yaml
46.0   requirements.txt
128    results
160    src

(notice stuff like 46.0)

and there doesn't seem to be a simple way to fix that in tqdm, in addition to the general feeling that we are abusing tqdm's util for this.

So let's just use humanize, which gets us a more pleasing result:

456B  .devcontainer.json
139B  .dvcignore
61B   .gitattributes
96B   .github
7B    .gitignore
5.8K  README.md
160B  data
1.8K  dvc.lock
519B  dvc.yaml
4B    foo
96B   models
96B   notebooks
225B  params.yaml
46B   requirements.txt
128B  results
160B  src

Followup for #9854

Currently we get odd formatting like

```
456    .devcontainer.json
139    .dvcignore
61.0   .gitattributes
96.0   .github
7.00   .gitignore
5.76k  README.md
160    data
1.84k  dvc.lock
519    dvc.yaml
4.00   foo
96.0   models
96.0   notebooks
225    params.yaml
46.0   requirements.txt
128    results
160    src
```

(notice stuff like 46.0)

and there doesn't seem to be a simple way to fix that in tqdm, in addition to
the general feeling that we are abusing tqdm's util for this.

So let's just use `humanize`, which gets us a more pleasing result:

```
456B  .devcontainer.json
139B  .dvcignore
61B   .gitattributes
96B   .github
7B    .gitignore
5.8K  README.md
160B  data
1.8K  dvc.lock
519B  dvc.yaml
4B    foo
96B   models
96B   notebooks
225B  params.yaml
46B   requirements.txt
128B  results
160B  src
```
@efiop efiop added the skip-changelog Skips changelog label Aug 17, 2023
@codecov
Copy link

codecov bot commented Aug 17, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.27% ⚠️

Comparison is base (2da959a) 90.84% compared to head (ea9309d) 90.57%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9857      +/-   ##
==========================================
- Coverage   90.84%   90.57%   -0.27%     
==========================================
  Files         471      471              
  Lines       35914    35914              
  Branches     5193     5193              
==========================================
- Hits        32625    32530      -95     
- Misses       2705     2778      +73     
- Partials      584      606      +22     
Files Changed Coverage Δ
dvc/commands/ls/__init__.py 75.47% <0.00%> (-1.89%) ⬇️

... and 26 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@efiop efiop merged commit 31ffce4 into iterative:main Aug 17, 2023
@skshetry
Copy link
Member

skshetry commented Aug 18, 2023

It’s one more dependency for a simple thing. I think it’s better to write our own.

Or, use something simple as:

def naturalsize(value: float, base: int = 1024) -> str:
    if value < base:
        return f"{value:.0f}"
    return tqdm.format_sizeof(value, divisor=base)

@efiop
Copy link
Contributor Author

efiop commented Aug 22, 2023

@skshetry That's fair, I'll do that in a separate PR. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-changelog Skips changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants