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

Use tqdm for better progress reporting during WC checkout #738

Merged
merged 1 commit into from
Nov 29, 2022

Conversation

olsen232
Copy link
Collaborator

Consistent, used for both vector and PC checkout, has some nice features:

  • don't need to explicitly log every 10th or 10,000th feature - tqdm decides.
  • logs to stderr, but keeps it brief if it is not isatty()
  • draws a nice progress bar

Still TODO, one day maybe: use tqdm during clone and fetch operations too

  • Have you reviewed your own change?
  • Have you included test(s)?
  • Have you updated the changelog?

Copy link
Member

@craigds craigds left a comment

Choose a reason for hiding this comment

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

Screen Shot 2022-11-29 at 12 08 53 PM

On wide screens it's a bit much; maybe limit to ncols=120 or so?

Otherwise 👍

kart/tabular/table_dataset.py Outdated Show resolved Hide resolved
kart/tabular/table_dataset.py Outdated Show resolved Hide resolved
@olsen232
Copy link
Collaborator Author

Setting ncols to a maximum is not done - currently, setting this explicitly seems to ruin tqdm's smart sizing (whereby it will make a smaller progress bar and cut out some cruft if the window is small). We can maybe get this fixed in tqdm one day, otherwise, current behaviour is better even if the progress bars look a bit extreme on a widescreen.

@craigds
Copy link
Member

craigds commented Nov 29, 2022

ok 👍
I think I misread the ncols doc as meaning a maximum column width, but it's actually an override for the autodetected terminal width.

Consistent, used for both vector and PC checkout, has some nice features:
- don't need to explicitly log every 10th or 10,000th feature - tqdm decides.
- logs to stderr, but keeps it brief if it is not isatty()
- draws a nice progress bar
Note that setting KART_SHOW_PROGRESS=1 will force it to output - useful for
clients running Kart as a subprocess that want to report progress.

Still TODO: use tqdm during clone and fetch operations too
@olsen232 olsen232 merged commit 8d5b024 into master Nov 29, 2022
@olsen232 olsen232 deleted the wc-progress-reporting branch November 29, 2022 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants