Skip to content

Fix pow() raising ValueError instead of TypeError for non-int exponent#7725

Merged
youknowone merged 2 commits into
RustPython:mainfrom
jseop-lim:compat/pow-error
Apr 29, 2026
Merged

Fix pow() raising ValueError instead of TypeError for non-int exponent#7725
youknowone merged 2 commits into
RustPython:mainfrom
jseop-lim:compat/pow-error

Conversation

@jseop-lim

@jseop-lim jseop-lim commented Apr 29, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Description

Fixes #7724

pow(x, y, z) raised ValueError when the modulus was zero even if the exponent was a non-integer such as None or float, instead of falling through to the TypeError that CPython raises.

AS-IS

>>> pow(1, None, 0)
ValueError: pow() 3rd argument cannot be 0

TO-BE

>>> pow(1, None, 0)
TypeError: unsupported operand type(s) for ** or pow(): 'int', 'NoneType', 'int'

(matching CPython)

Changes

  • Downcast other to PyInt first in PyInt::modpow and return NotImplemented when the cast fails, so ternary_op falls through to the proper TypeError — either the formatted "unsupported operand type(s)" message or PyFloat's "pow() 3rd argument not allowed unless all arguments are integers".
  • Add regression tests in extra_tests/snippets/builtin_pow.py covering pow(1, None, 0) and pow(True, 1.5, False).

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Enhanced type validation in the pow() function's 3-argument form to properly handle and reject incompatible argument types.

  • Tests

    • Added test cases verifying pow() raises TypeError for incompatible argument types.

@coderabbitai

coderabbitai Bot commented Apr 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 1f8e3be6-170a-4d9e-8cac-6e50cf22548a

📥 Commits

Reviewing files that changed from the base of the PR and between 3f718f9 and 0d50b8c.

📒 Files selected for processing (2)
  • crates/vm/src/builtins/int.rs
  • extra_tests/snippets/builtin_pow.py
📝 Walkthrough

Walkthrough

The changes fix the validation order in PyInt::modpow to check that the exponent operand is a PyInt before validating the modulus value, ensuring TypeError is raised for incompatible types instead of ValueError, and adds test cases to verify the fix.

Changes

Cohort / File(s) Summary
Type validation fix
crates/vm/src/builtins/int.rs		 Added explicit downcast check for other operand to PyInt before zero-modulus validation, returning not_implemented if the cast fails to ensure proper error propagation.
Test coverage
extra_tests/snippets/builtin_pow.py		 Added two negative test cases verifying that pow with 3 arguments raises TypeError when the exponent or modulus has incompatible types.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

A rabbit reorders checks with care, 🐰
Type before zero, fair and square,
TypeError blooms where it should be,
Modular power flows naturally!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and accurately describes the main change: fixing pow() to raise TypeError instead of ValueError for non-int exponent cases.
Linked Issues check ✅ Passed The PR implements the exact fix described in issue #7724: downcasting the exponent operand to PyInt first and returning NotImplemented before checking zero-modulus, allowing proper TypeError dispatch.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the pow() exponent type-checking order and adding regression tests; no unrelated modifications are present.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.

Comment @coderabbitai help to get the list of available commands and usage tips.

@jseop-lim

jseop-lim commented Apr 29, 2026

Copy link
Copy Markdown
Contributor Author

I'm still fairly new to both Rust and RustPython, so I appreciate your patience. Please feel free to point out anything that could be improved in this request:)

ShaharNaveh
ShaharNaveh approved these changes Apr 29, 2026
View reviewed changes

@ShaharNaveh ShaharNaveh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ty:)

@ShaharNaveh

ShaharNaveh commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

I'm still fairly new to both Rust and RustPython, so I appreciate your patience. Please feel free to point out anything that could be improved in this request:)

Welcome to the project 🥳

youknowone
youknowone approved these changes Apr 29, 2026
View reviewed changes

@youknowone youknowone left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

@youknowone youknowone merged commit c8ddbd2 into RustPython:main Apr 29, 2026
36 of 38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@youknowone youknowone youknowone approved these changes

@ShaharNaveh ShaharNaveh ShaharNaveh approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

pow(1, None, 0) raises ValueError instead of TypeError

3 participants

@jseop-lim @ShaharNaveh @youknowone