Skip to content

tqdm 5 #13

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.

Sign up for GitHub

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

Open
v4rgas wants to merge 1 commit into
base: main
Choose a base branch
from
Open

tqdm 5 #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions tqdm_5/tqdm/_tqdm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -748,19 +748,12 @@ def __init__(self, iterable=None, desc=None, total=None, leave=True,
if disable is None and hasattr(file, "isatty") and not file.isatty():
disable = True

if total is None and iterable is not None:
try:
total = len(iterable)
except (TypeError, AttributeError):
total = None

Copy link

@blar-app blar-app bot May 23, 2025

Choose a reason for hiding this comment

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

🟠 Warning 🐛 Bug

AttributeError can occur if self.total is not set for disabled tqdm instances.

Issue Explanation
  • In the __init__ method, the assignment self.total = total was removed inside the disable branch.
  • As a result, disabled tqdm instances (with disable=True) might not have self.total initialized.
  • The __len__ method accesses self.total without a fallback if self.iterable is None, which will raise AttributeError.
  • This confirms that the removal of self.total in the disable branch is a regression causing potential runtime errors.

Reply if you have any questions or let me know if I missed something.

Don't forget to react with a 👍 or 👎 to the comments made by Blar to help us improve.

if disable:
self.iterable = iterable
self.disable = disable
self.pos = self._get_free_pos(self)
self._instances.remove(self)
self.n = initial
self.total = total
return

if kwargs:
Expand All @@ -773,6 +766,12 @@ def __init__(self, iterable=None, desc=None, total=None, leave=True,
else TqdmKeyError("Unknown argument(s): " + str(kwargs)))

# Preprocess the arguments
if total is None and iterable is not None:
try:
total = len(iterable)
except (TypeError, AttributeError):
total = None

Comment on lines +769 to +774
Copy link

@coderabbitai coderabbitai bot May 23, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

LGTM! The total inference logic repositioning improves initialization flow.

The refactoring to move total inference after argument validation is a logical improvement. The code correctly handles the case where total is not explicitly provided by attempting to derive it from the iterable's length, with proper exception handling for objects that don't support len().

This change ensures that argument validation occurs before any processing, which is a better practice and can improve performance in error cases.

However, since this changes the initialization order and the AI summary mentions that related test coverage was removed, let's verify that this doesn't break existing functionality:

🏁 Script executed:

#!/bin/bash
# Description: Search for any code that might depend on total being set early in initialization

# Search for any references to self.total in early return paths or disabled cases
rg -A 5 -B 5 "self\.total.*disable" 

# Search for any tests or code that might depend on total being available when disabled
rg -A 10 "disable.*total|total.*disable"

# Search for any boolean evaluation tests that might have been affected
rg -A 5 -B 5 "test.*bool|bool.*test"

Length of output: 21613

🏁 Script executed:

#!/bin/bash
# Verify boolean special methods and test presence in tqdm_5

# Search for __bool__ method
rg -n "def __bool__" tqdm_5/tqdm/_tqdm.py

# Search for __nonzero__ method (Py2 compatibility)
rg -n "def __nonzero__" tqdm_5/tqdm/_tqdm.py

# Check if boolean evaluation test exists in version 5
grep -R "def test_bool" -n tqdm_5/tqdm/tests/tests_tqdm.py

Length of output: 151

Boolean evaluation support missing in tqdm class

The recent refactoring removed the __bool__/__nonzero__ methods and the corresponding test_bool cases, so bool(tqdm(...)) will always be True (even when disabled or zero-length). This changes the public API and breaks existing user patterns.

Please restore boolean‐casting behavior:

• In tqdm_5/tqdm/_tqdm.py (class tqdm), reintroduce:

def __bool__(self):
    return not self.disable and (self.total is None or self.total != 0)
__nonzero__ = __bool__  # for Py2 compatibility

• In tqdm_5/tqdm/tests/tests_tqdm.py, re-add and adapt the test_bool suite from v4/v3 to ensure:

  • bool(tqdm(total=0, disable=False)) is False
  • bool(tqdm(total>0, disable=False)) is True
  • bool(tqdm(..., disable=True)) is False

This will preserve backward‐compatible truthiness semantics.

🤖 Prompt for AI Agents 
In tqdm_5/tqdm/_tqdm.py around lines 769 to 774, the boolean evaluation methods
__bool__ and __nonzero__ were removed, breaking the expected truthiness behavior
of tqdm instances. Reintroduce the __bool__ method to return False when disabled
or total is zero, and True otherwise, and alias __nonzero__ to __bool__ for
Python 2 compatibility. Additionally, in tqdm_5/tqdm/tests/tests_tqdm.py,
restore and adapt the test_bool test suite to verify that bool(tqdm(total=0,
disable=False)) is False, bool(tqdm(total>0, disable=False)) is True, and
bool(tqdm(..., disable=True)) is False, ensuring backward-compatible boolean
semantics.

if ((ncols is None) and (file in (sys.stderr, sys.stdout))) or \
dynamic_ncols: # pragma: no cover
if dynamic_ncols:
Expand Down
31 changes: 0 additions & 31 deletions tqdm_5/tqdm/tests/tests_tqdm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1560,37 +1560,6 @@ def test_threading():
# TODO: test interleaved output #445


@with_setup(pretest, posttest)
def test_bool():
"""Test boolean cast"""
def internal(our_file, disable):
with tqdm(total=10, file=our_file, disable=disable) as t:
assert t
with tqdm(total=0, file=our_file, disable=disable) as t:
assert not t
with trange(10, file=our_file, disable=disable) as t:
assert t
with trange(0, file=our_file, disable=disable) as t:
assert not t
with tqdm([], file=our_file, disable=disable) as t:
assert not t
with tqdm([0], file=our_file, disable=disable) as t:
assert t
with tqdm(file=our_file, disable=disable) as t:
try:
print(bool(t))
except TypeError:
pass
else:
raise TypeError(
"Expected tqdm() with neither total nor iterable to fail")

# test with and without disable
with closing(StringIO()) as our_file:
internal(our_file, False)
internal(our_file, True)

Comment on lines -1563 to -1592
Copy link

@blar-app blar-app bot May 23, 2025

Choose a reason for hiding this comment

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

🔴 Error 🐛 Bug

Removal of test enforcing TypeError for bool(tqdm()) with no total or iterable.

Issue Explanation
  • The test test_bool() in tests_tqdm.py previously checked that calling bool() on a tqdm instance without total or iterable raised a TypeError.
  • This test case has been removed entirely from the test suite as part of the changes.
  • The removal means there is no longer test coverage ensuring that bool(tqdm()) raises a TypeError under these conditions.
  • The actual behavior change (whether bool(tqdm()) now returns a value or silently passes) is not confirmed, but the coverage gap is clear.
  • This could lead to altered or unexpected behavior when bool() is called on a tqdm instance constructed without arguments.

Reply if you have any questions or let me know if I missed something.

Don't forget to react with a 👍 or 👎 to the comments made by Blar to help us improve.


@with_setup(pretest, posttest)
def test_autonotebook():
"""Test autonotebook fallback"""
Expand Down