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

tqdm: fix casting to boolean to mimic self.iterable #694

Merged

Conversation

immerrr
Copy link
Contributor

@immerrr immerrr commented Mar 6, 2019

I have hit an issue described in #353 today with what can be boiled down to:

In [28]: bool(tqdm.tqdm((1 for i in range(10))))                                                                                                                                              
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-28-e62055aa18f4> in <module>
----> 1 bool(tqdm.tqdm((1 for i in range(10))))

TypeError: 'NoneType' object cannot be interpreted as an integer

In [29]:   import tqdm, sys 
    ...:   print(tqdm.__version__, sys.version, sys.platform)                                                                                                                                 
4.29.1 3.6.6 (default, Oct  9 2018, 14:19:21) 
[GCC 5.4.0 20160609] linux

This is a stab at fixing it: it will make the tqdm wrapper behave in exactly the same way as the wrapped self.iterable when cast to boolean. Admittedly, it could be made more intelligent by comparing the current position to the total length if it is known, but that would complicate things and I wanted to start with a simple solution.

  • I have visited the source website, and in particular
    read the known issues
  • I have searched through the issue tracker for duplicates
  • I have mentioned version numbers, operating system and
    environment, where applicable:
    import tqdm, sys
    print(tqdm.__version__, sys.version, sys.platform)
  • If applicable, I have mentioned the relevant/related issue(s)

@immerrr immerrr force-pushed the fix-tqdm-error-for-iterators-in-bool-context branch 2 times, most recently from 2699f0c to 35b701d Compare March 6, 2019 14:07
@immerrr
Copy link
Contributor Author

immerrr commented Mar 6, 2019

I have reviewed the failures and realized that I somehow missed the test_bool function that already tests the boolean cast behaviour that prioritizes self.total over the boolean cast for self.iterable, so I have pushed a change to __bool__ operator that does that.

The existing test also tests for TypeError if the tqdm has no self.iterable, but doesn't test it for sizeless iterators, so I have adjusted my PR to play nice with the existing expectations.

@immerrr immerrr force-pushed the fix-tqdm-error-for-iterators-in-bool-context branch 2 times, most recently from bc69e04 to f07436a Compare March 6, 2019 14:17
@codecov-io
Copy link

codecov-io commented Mar 6, 2019

Codecov Report

Merging #694 into devel will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##            devel    #694      +/-   ##
=========================================
+ Coverage   98.58%   98.6%   +0.01%     
=========================================
  Files          10      10              
  Lines         779     787       +8     
  Branches      141     143       +2     
=========================================
+ Hits          768     776       +8     
  Misses          6       6              
  Partials        5       5

@casperdcl casperdcl self-assigned this Mar 6, 2019
@casperdcl casperdcl added the to-merge ↰ Imminent label Mar 6, 2019
@casperdcl
Copy link
Sponsor Member

thanks.

@casperdcl casperdcl force-pushed the fix-tqdm-error-for-iterators-in-bool-context branch from f07436a to 73962a4 Compare May 16, 2019 18:55
@ghost ghost added the review label May 16, 2019
@casperdcl casperdcl changed the base branch from master to devel May 16, 2019 18:56
@casperdcl casperdcl merged commit 73962a4 into tqdm:devel May 16, 2019
@ghost ghost removed the review label May 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants