Skip to content

Conversation

chengs
Copy link
Contributor

@chengs chengs commented Apr 12, 2018

@casperdcl
I think here it should be a warning, because normally total should be automatically computed by tqdm. and only a few cases need manually set total. Like this:
screen shot 2018-04-12 at 12 16 11 pm

@codecov-io
Copy link

codecov-io commented Apr 12, 2018

Codecov Report

Merging #535 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master     #535   +/-   ##
=======================================
  Coverage   99.41%   99.41%           
=======================================
  Files           8        8           
  Lines         681      681           
  Branches      118      118           
=======================================
  Hits          677      677           
  Misses          3        3           
  Partials        1        1

@chengs chengs requested a review from casperdcl April 12, 2018 10:25
Fix #539 where `iterable is not None and not hasattr(iterable, "__len__")`
casperdcl pushed a commit that referenced this pull request Apr 15, 2018
@casperdcl
Copy link
Member

casperdcl commented Apr 15, 2018

@chengs I don't think the warning is a good idea - if people specify their own total they probably know what they're doing

@casperdcl
Copy link
Member

thanks for the unit tests - I left them in.

@casperdcl casperdcl changed the title Add total support to tqdm.pandas() to fix #364 Add total support to tqdm.pandas() Apr 15, 2018
@casperdcl casperdcl added p3-enhancement 🔥 Much new such feature submodule ⊂ Periphery/subclasses labels Apr 15, 2018
@chengs
Copy link
Contributor Author

chengs commented Apr 15, 2018

@casperdcl, about warning, OK. you make the final decision, so no warning then.

Copy link
Contributor Author

@chengs chengs left a comment

Choose a reason for hiding this comment

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

two redundant lines, but it is ok

@casperdcl
Copy link
Member

Redundant lines? Which ones? Are they still in devel?

@chengs
Copy link
Contributor Author

chengs commented Apr 15, 2018

@casperdcl no, the devel is good. Ok to go.

@casperdcl casperdcl merged commit a432c3d into tqdm:master Apr 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p3-enhancement 🔥 Much new such feature submodule ⊂ Periphery/subclasses
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants