Skip to content

Conversation

@cclauss
Copy link

@cclauss cclauss commented Nov 28, 2025

lint.extend-select = [
  "ASYNC",
  "C4",
  "PERF",
]

https://docs.astral.sh/ruff

Extra review please on the time.sleep() --> asyncio.sleep() in bindings/python/tests/bindings/test_tokenizer.py because these are tests, not production code.


% cd bindings/python
% ruff check --extend-select=ASYNC,C4,PERF --output-format=concise

benches/test_tiktoken.py:32:11: F541 [*] f-string without any placeholders
examples/using_the_visualizer.ipynb:cell 13:1:21: C408 Unnecessary `dict()` call (rewrite as a literal)
py_src/tokenizers/tools/visualizer.py:167:18: C417 Unnecessary `map()` usage (rewrite using a set comprehension)
scripts/sentencepiece_extractor.py:141:37: C417 Unnecessary `map()` usage (rewrite using a generator expression)
tests/bindings/test_tokenizer.py:344:20: C419 Unnecessary list comprehension
tests/bindings/test_tokenizer.py:953:17: ASYNC251 Async functions should not call `time.sleep`
tests/bindings/test_tokenizer.py:964:17: ASYNC251 Async functions should not call `time.sleep`
Found 7 errors.

% ruff rule ASYNC251

blocking-sleep-in-async-function (ASYNC251)

Derived from the flake8-async linter.

What it does

Checks that async functions do not call time.sleep.

Why is this bad?

Blocking an async function via a time.sleep call will block the entire
event loop, preventing it from executing other tasks while waiting for the
time.sleep, negating the benefits of asynchronous programming.

Instead of time.sleep, use asyncio.sleep.

Example

import time


async def fetch():
    time.sleep(1)

Use instead:

import asyncio


async def fetch():
    await asyncio.sleep(1)

Copy link
Collaborator

@ArthurZucker ArthurZucker left a comment

Choose a reason for hiding this comment

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

sure why not!

Comment on lines +56 to +59
lint.extend-select = [
"ASYNC",
"C4",
"PERF",
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you explain this?

Copy link
Author

@cclauss cclauss Nov 28, 2025

Choose a reason for hiding this comment

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

We are adding these three families of rules to the ruff linter. The rules are documented in the URLs in the commit message.

https://docs.astral.sh/ruff/settings/#lint_extend-select

@cclauss cclauss requested a review from ArthurZucker November 28, 2025 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants