Skip to content

tqdm 4 #12

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 4 #12

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
3 changes: 1 addition & 2 deletions tqdm_4/tqdm/_tqdm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,8 +320,7 @@ def format_meter(n, total, elapsed, ncols=None, prefix='', ascii=False,

# apply custom scale if necessary
if unit_scale and unit_scale not in (True, 1):
if total:
total *= unit_scale
Comment on lines -323 to -324
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

Always scaling total without checking for None triggers a TypeError when unit_scale is a numeric value.

Issue Explanation
  • The PR removed the if total: guard before applying custom scaling.
  • When unit_scale is a numeric value other than True or 1, the block always executes total *= unit_scale.
  • If total is None, this multiplication raises a TypeError (unsupported operand type(s) for *=: 'NoneType' and 'int').
  • The function API treats total as None for unknown or unset totals, so this scenario is valid and must be handled.
  • See blarApp/open-benchmark/tqdm_4/tqdm/_tqdm.py → format_meter (apply custom scale section).
# apply custom scale if necessary
if unit_scale and unit_scale not in (True, 1):
    total *= unit_scale

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.

total *= unit_scale
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.

⚠️ Potential issue

Critical Issue: TypeError when total is None

This unconditional multiplication will raise a TypeError when total is None (which occurs when the total number of iterations is unknown) and unit_scale is a numeric value.

Apply this fix to restore the conditional check:

-            total *= unit_scale
+            if total is not None:
+                total *= unit_scale

Alternatively, use a more defensive approach:

-            total *= unit_scale
+            total = total * unit_scale if total is not None else None
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
total *= unit_scale
if unit_scale and unit_scale is not True and unit_scale != 1:
- total *= unit_scale
+ if total is not None:
+ total *= unit_scale
n = int(round(n * unit_scale))
🤖 Prompt for AI Agents 
In tqdm_4/tqdm/_tqdm.py at line 323, the code multiplies total by unit_scale
without checking if total is None, causing a TypeError when total is None. Fix
this by adding a conditional check to ensure total is not None before performing
the multiplication, preventing the TypeError when total is unknown.

n *= unit_scale
if rate:
rate *= unit_scale # by default rate = 1 / self.avg_time
Expand Down
9 changes: 0 additions & 9 deletions tqdm_4/tqdm/tests/tests_tqdm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -772,15 +772,6 @@ def test_infinite_total():
pass


@with_setup(pretest, posttest)
def test_nototal():
"""Test unknown total length"""
with closing(StringIO()) as our_file:
for i in tqdm((i for i in range(10)), file=our_file, unit_scale=10):
pass
assert "100it" in our_file.getvalue()

Comment on lines 774 to -782
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.

🟡 Info 🐛 Bug

Removal of test_nototal eliminates coverage for iterables without a predefined total and unit_scale=10 scaling behavior.

Issue Explanation
  • The removed test_nototal function ensured tqdm correctly scales iteration counts by unit_scale on unknown-length iterables.
  • Without this test, regressions in scaling logic (e.g., producing "100it") will go undetected.
  • The snippet in blarApp/open-benchmark/tqdm_4/tqdm/tests/tests_tqdm.py no longer verifies that a generator with unit_scale=10 outputs "100it".
for i in tqdm((i for i in range(10)), file=our_file, unit_scale=10):
    pass
assert "100it" in our_file.getvalue()

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_unit():
"""Test SI unit prefix"""
Expand Down