Replace clint by tqdm for progressbar by Carreau · Pull Request #242 · pypa/twine

Carreau · 2017-04-27T21:05:26Z

Closes #241 – As clint as some indirect Python 2 dependency and tqdm has
not.

sethmlarson · 2017-04-27T22:27:41Z

            encoder = MultipartEncoder(data_to_send)
-            bar = ProgressBar(expected_size=encoder.len, filled_char='=')
+            bar = ProgressBar(total=encoder.len, unit='bytes', unit_scale=True, leave=False)
+            def update_progressbar(monitor, _cache=[0]):


This relies on a little Python quirk that isn't as intuitive. Thoughts on factoring this out into it's own object and then passing a method of that object to MultipartEncoderMonitor?

Why not use a variable that lives at the same scope as the function instead of relying on this. I agree with @SethMichaelLarson that this isn't going to be easy to maintain.

I created a custom subclass of tqdm with a update_to method.

sigmavirus24 · 2017-04-27T22:35:34Z


 install_requires = [
-    "clint",
+    "tqdm",


Please add a lower limit here.

sigmavirus24 · 2017-04-27T22:37:15Z

            encoder = MultipartEncoder(data_to_send)
-            bar = ProgressBar(expected_size=encoder.len, filled_char='=')
+            bar = ProgressBar(total=encoder.len, unit='bytes', unit_scale=True, leave=False)
+            def update_progressbar(monitor, _cache=[0]):


Why not use a variable that lives at the same scope as the function instead of relying on this. I agree with @SethMichaelLarson that this isn't going to be easy to maintain.

sigmavirus24 · 2017-04-28T22:51:07Z

 [metadata]
 requires-dist =
-    clint
+    tqdm


Can you have this match the version in install_requires. I'd really rather not use newer features of very recent versions of setuptools, e.g., ~=. Can we just use >=, please?

This needs to match setup.py. When people install from a wheel, we want the requirements to match those that they see when installing from an sdist.

sigmavirus24 · 2017-04-28T22:51:43Z

+        """
+        identical to update, except `n` should be current value and not delta.
+        """
+        self.update(n-self.n)


I'm disappointed Flake8 didn't catch this, but can you add spaces around -?

Carreau · 2017-04-29T17:58:28Z

Done and done. I did not realized that ~= was that recent as the pep 440 is from 2013.

sigmavirus24 · 2017-04-29T21:55:15Z

No worries. All I know is that even last year, setuptools didn't support it as well as it could. Plus setuptools is one of those things people don't ever consider upgrading unless they run into issues, so there's such a wide swath of versions that we need to avoid using features that are too new.

sigmavirus24 · 2017-04-29T22:30:54Z

There are still some Flake8 warnings that need cleaning up. https://travis-ci.org/pypa/twine/jobs/227188445#L201 You can find them locally with tox -e pep8. Thanks again for tackling this! 🍰

Carreau · 2017-05-01T15:39:38Z

pep8ified.

sethmlarson · 2017-05-01T18:13:42Z

 from __future__ import absolute_import, unicode_literals, print_function

-from clint.textui.progress import Bar as ProgressBar
+from tqdm import tqdm as tqdm


Pretty sure this has no effect. Should be:

from tqdm import tqdm

sethmlarson · 2017-05-01T18:15:09Z

                headers={'Content-Type': monitor.content_type},
            )
-            bar.done()
+            bar.close()


Can tqdm.tqdm be used as a context manager? If so it would be nice to use it like one so that .close() is always called, even on exception.

If not it might be worth it to do something like this:

bar = ProgressBar(total=encoder.len, unit='bytes', unit_scale=True, leave=False) try: # Setup monitor and response here. finally: bar.close()

https://pypi.python.org/pypi/tqdm#manual leads me to believe that in fact tqdm can be used as a context manager.

Personally, I'd rather this look like it does except that we do:

with bar: resp = self.session.post( # ... )

Because I'd rather keep the scope of the context-manager small.

sigmavirus24 · 2017-05-02T12:39:38Z

 [metadata]
 requires-dist =
-    clint
+    tqdm


This needs to match setup.py. When people install from a wheel, we want the requirements to match those that they see when installing from an sdist.

sigmavirus24 · 2017-05-02T12:41:32Z

                headers={'Content-Type': monitor.content_type},
            )
-            bar.done()
+            bar.close()


https://pypi.python.org/pypi/tqdm#manual leads me to believe that in fact tqdm can be used as a context manager.

Personally, I'd rather this look like it does except that we do:

with bar: resp = self.session.post( # ... )

Because I'd rather keep the scope of the context-manager small.

sigmavirus24 · 2017-05-02T12:42:32Z

@Carreau for what it's worth, if this is more work than you'd like to complete, let me know and I'll add the final polishing touches for you.

Closes pypa#241 – As clint as some indirect Python 2 dependency and tqdm has not.

Carreau · 2017-05-02T17:23:14Z

@Carreau for what it's worth, if this is more work than you'd like to complete, let me know and I'll add the final polishing touches for you.

No it's ok, thanks for the proposal ! Now contexmanagerified, and setup.cfg fixed.

sigmavirus24 · 2017-05-14T16:29:35Z

Thanks @Carreau!! 🍰

Class-based, inspired by [twine#242](pypa/twine#242), [here](pypa/twine@42e55e06).

Carreau mentioned this pull request Apr 27, 2017

Twine have indirect Python 2 only dependency. #241

Closed

sethmlarson reviewed Apr 27, 2017

View reviewed changes

sigmavirus24 requested changes Apr 27, 2017

View reviewed changes

sigmavirus24 requested changes Apr 28, 2017

View reviewed changes

Carreau force-pushed the clint-to-tqdm branch from 38e3169 to f3b7492 Compare April 29, 2017 17:56

Carreau force-pushed the clint-to-tqdm branch from f3b7492 to 03bf142 Compare May 1, 2017 15:28

sethmlarson reviewed May 1, 2017

View reviewed changes

sigmavirus24 requested changes May 2, 2017

View reviewed changes

Carreau force-pushed the clint-to-tqdm branch 2 times, most recently from 18ce611 to 42e55e0 Compare May 2, 2017 17:22

Replace clint by tqdm for progressbar

42e55e0

Closes pypa#241 – As clint as some indirect Python 2 dependency and tqdm has not.

sigmavirus24 merged commit 42e55e0 into pypa:master May 14, 2017

casperdcl added a commit to tqdm/tqdm that referenced this pull request May 29, 2017

update to OOP urllib hook example

c1d56bf

Class-based, inspired by [twine#242](pypa/twine#242), [here](pypa/twine@42e55e06).

casperdcl mentioned this pull request May 29, 2017

update and tweak tqdm progressbar #256

Merged

7 tasks

casperdcl mentioned this pull request Apr 8, 2018

recognition tqdm/tqdm#247

Open

8 tasks

monogege added a commit to monogege/deviations that referenced this pull request Sep 6, 2025

update to OOP urllib hook example

c5ac7c3

Class-based, inspired by [twine#242](pypa/twine#242), [here](pypa/twine@42e55e06).

MarekWave added a commit to MarekWave/blockchain_learn_web3 that referenced this pull request Oct 8, 2025

update to OOP urllib hook example

2e0ad92

Class-based, inspired by [twine#242](pypa/twine#242), [here](pypa/twine@42e55e06).

defaultnamethere added a commit to defaultnamethere/defaultnamethere that referenced this pull request Nov 28, 2025

update to OOP urllib hook example

e0986b0

Class-based, inspired by [twine#242](pypa/twine#242), [here](pypa/twine@42e55e06).

Conversation

Carreau commented Apr 27, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Carreau commented Apr 29, 2017

Uh oh!

sigmavirus24 commented Apr 29, 2017

Uh oh!

sigmavirus24 commented Apr 29, 2017

Uh oh!

Carreau commented May 1, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sigmavirus24 commented May 2, 2017

Uh oh!

Carreau commented May 2, 2017

Uh oh!

sigmavirus24 commented May 14, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants