Skip to content

typing: allow integer in bitwise operations #155704

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

typing: allow integer in bitwise operations #155704

wants to merge 1 commit into from

Conversation

randolf-scholz
Copy link
Contributor

@randolf-scholz randolf-scholz commented Jun 11, 2025 •
edited by pytorch-bot bot
Loading

Fixes #155701 (false positives)

cc @ezyang @malfet @xuzhao9 @gramster

@pytorch-bot PyTorch Bot
Copy link

pytorch-bot bot commented Jun 11, 2025 •
edited
Loading

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/155704

Note: Links to docs will display an error until the docs builds have been completed.

✅ You can merge normally! (2 Unrelated Failures)

As of commit 5972b24 with merge base 013cf1e (image):

BROKEN TRUNK - The following job failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

UNSTABLE - The following job is marked as unstable, possibly due to flakiness on trunk:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@pytorch-bot pytorch-bot bot added the topic: not user facing topic category label Jun 11, 2025
@randolf-scholz
Copy link
Contributor Author

@pytorchbot label "module: typing"

@pytorch-bot pytorch-bot bot added the module: typing Related to mypy type annotations label Jun 11, 2025
@randolf-scholz
Copy link
Contributor Author

The false negatives seems to originate from autogenerated function signatures from C++ code.

@Skylion007 Skylion007 requested a review from aorenste June 11, 2025 14:58
@Skylion007 Skylion007 added release notes: python_frontend python frontend release notes category and removed topic: not user facing topic category labels Jun 11, 2025
aorenste
aorenste reviewed Jun 11, 2025
View reviewed changes
test/typing/fail/arithmetic_ops.py
^ TENSOR,
Tensor,
)
FLOAT & TENSOR # E: Unsupported operand types for & ("float" and "Tensor")
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the thinking here? Do we not want to check the result type because it doesn't really make sense to check the type of an unsupported op?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes

aorenste
aorenste reviewed Jun 11, 2025
View reviewed changes
tools/pyi/gen_pyi.py
return [f"def {opname}(self, other: Tensor | Number | _complex) -> Tensor: ..."]
elif name in logic_ops:
return [f"def {opname}(self, other: Tensor | _bool) -> Tensor: ..."]
return [f"def {opname}(self, other: Tensor | _int) -> Tensor: ..."]
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this include bool?

Suggested change
return [f"def {opname}(self, other: Tensor | _int) -> Tensor: ..."]
return [f"def {opname}(self, other: Tensor | _bool | _int) -> Tensor: ..."]

This is valid code:

bool_tensor1 = torch.tensor([True, False, True, False], dtype=torch.bool)
print(bool_tensor1 & False)

Copy link
Contributor Author

@randolf-scholz randolf-scholz Jun 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

builtins.bool is a subclass of builtins.int, perhaps somewhat surprisingly. So adding bool here would only serve documentation purposes (which can be a good reason), but wouldn't change the type inference.

From a typing POV it can be somewhat unfortunate that bool is a subclass of int, here is a longer discussion on the topic: https://discuss.python.org/t/is-there-a-road-towards-making-bool-a-fully-fledged-boolean-type/62392

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you check the tests, this behavior is already covered.

pytorch/test/typing/pass/arithmetic_ops.py

Lines 74 to 76 in 5972b24

assert_type(BOOL & TENSOR, Tensor)
assert_type(BOOL | TENSOR, Tensor)
assert_type(BOOL ^ TENSOR, Tensor)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah - I had just noticed that the tests covered and was trying to understand why there wasn't an error. The subclass explains it.

aorenste
aorenste approved these changes Jun 11, 2025
View reviewed changes
tools/pyi/gen_pyi.py
return [f"def {opname}(self, other: Tensor | Number | _complex) -> Tensor: ..."]
elif name in logic_ops:
return [f"def {opname}(self, other: Tensor | _bool) -> Tensor: ..."]
return [f"def {opname}(self, other: Tensor | _int) -> Tensor: ..."]
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah - I had just noticed that the tests covered and was trying to understand why there wasn't an error. The subclass explains it.

@Skylion007
Copy link
Collaborator

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Jun 12, 2025
@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: 1 jobs have failed, first few of them are: trunk / macos-py3-arm64 / test (mps, 1, 1, macos-m1-14)

Details for Dev Infra team Raised by workflow job

@Skylion007
Copy link
Collaborator

@pytorchbot merge -i

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged while ignoring the following 3 checks: pull / linux-jammy-py3-clang12-executorch / test (executorch, 1, 1, linux.2xlarge, unstable), pull / cuda12.8-py3.10-gcc9-sm75 / test (pr_time_benchmarks, 1, 1, linux.g4dn.metal.nvidia.gpu), trunk / macos-py3-arm64 / test (mps, 1, 1, macos-m1-14)

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

thatgeeman pushed a commit to thatgeeman/pytorch-docathon that referenced this pull request Jun 15, 2025
@randolf-scholz @thatgeeman
typing: allow integer in bitwise operations (pytorch#155704)
a8b937d 
Fixes pytorch#155701 (false positives)

Pull Request resolved: pytorch#155704
Approved by: https://github.com/Skylion007, https://github.com/aorenste
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Skylion007 Skylion007 Skylion007 approved these changes

@aorenste aorenste aorenste approved these changes

Assignees

No one assigned

Labels

ciflow/trunk Trigger trunk jobs on your pull request Merged module: typing Related to mypy type annotations open source release notes: python_frontend python frontend release notes category

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Typing: Incorrect overload for boolean operators

5 participants

@randolf-scholz @Skylion007 @pytorchmergebot @aorenste @pytorchbot