Skip to content

fix: replace assert with proper exception in python_build_standalone#2859

Open
henryiii wants to merge 1 commit into
pypa:mainfrom
henryiii:henryiii/fix/assert-runtime-validation
Open

fix: replace assert with proper exception in python_build_standalone#2859
henryiii wants to merge 1 commit into
pypa:mainfrom
henryiii:henryiii/fix/assert-runtime-validation

Conversation

@henryiii
Copy link
Copy Markdown
Contributor

@henryiii henryiii commented May 16, 2026

See #2854.

🤖 Human triggered, AI assisted PR (using this skill). AI text below. 🤖

Summary

Replaces assert not python_base_dir.exists() in cibuildwheel/util/python_build_standalone.py:178 with an explicit check that raises PythonBuildStandaloneError. Assert statements are skipped under python -O, making them unsuitable for runtime validation.

Identified in #2854 ("Uses assert for runtime validation — It is skipped with python -O").

Assisted-by: OpenCode:glm-5

Assert statements are skipped under python -O, making them unsuitable
for runtime validation. Replace assert with an explicit check that
raises PythonBuildStandaloneError if the target directory already exists.

Assisted-by: OpenCode:glm-5
Copy link
Copy Markdown
Member

@agriyakhetarpal agriyakhetarpal left a comment

Choose a reason for hiding this comment

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

Thanks! I think it might be a good idea to add S101: https://docs.astral.sh/ruff/rules/assert/

@joerick
Copy link
Copy Markdown
Contributor

joerick commented May 16, 2026

I'd be opposed to S101 - asserts are mighty useful to check for programmer mistakes, and handy to tell the type system something that you as a programmer know to be true.

@agriyakhetarpal
Copy link
Copy Markdown
Member

@henryiii
Copy link
Copy Markdown
Contributor Author

Assert is used to narrow type checkers. The AI was able to detect a usage that was not for type checking, but the general ruff check can't do that.

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.

3 participants