Skip to content
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.

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

PYI055 suggestion causes TypeError at runtime #6455

Closed
adamtheturtle opened this issue Aug 9, 2023 · 3 comments · Fixed by #6457
Closed

PYI055 suggestion causes TypeError at runtime #6455

adamtheturtle opened this issue Aug 9, 2023 · 3 comments · Fixed by #6457
Assignees
Labels
bug Something isn't working

Comments

@adamtheturtle
Copy link

ruff 0.0.283 (error is new in this version)
Python 3.11.4

I get an error from ruff which proposes a solution, and applying that proposal causes an error.

I have the following file:

# example.py
#
# pip install requests_mock httpretty

import httpretty
import requests_mock

item: type[requests_mock.Mocker] | type[httpretty] = requests_mock.Mocker
  • python example.py has no output
  • ruff --select=PYI --isolated example.py shows:
example.py:42:7: PYI055 Multiple `type` members in a union. Combine them into one, e.g., `type[requests_mock.Mocker | httpretty]`.

The ruff documentation at https://beta.ruff.rs/docs/rules/unnecessary-type-union/ suggests that this change is purely cosmetic:

The type built-in function accepts unions, and it is clearer to explicitly specify them as a single type.

However, if I make the change suggested, and then run python example.py, I get a TypeError:

Traceback (most recent call last):
  File "/Users/adam/Desktop/mypy-ruff-example/example.py", line 38, in <module>
    item: type[requests_mock.Mocker | httpretty] = requests_mock.Mocker
               ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
TypeError: unsupported operand type(s) for |: 'type' and 'module'
@charliermarsh charliermarsh added the bug Something isn't working label Aug 9, 2023
@charliermarsh
Copy link
Member

Thanks.

@charliermarsh
Copy link
Member

(I think you mean PYI055.)

@zanieb zanieb changed the title PYI030 suggestion causes TypeError at runtime PYI055 suggestion causes TypeError at runtime Aug 9, 2023
@charliermarsh charliermarsh self-assigned this Aug 9, 2023
charliermarsh added a commit that referenced this issue Aug 9, 2023
## Summary

The use of `|` as a union operator is not always safe, if a type
annotation is evaluated in a runtime context. For example, this code
errors at runtime:

```python
import httpretty
import requests_mock

item: type[requests_mock.Mocker | httpretty] = requests_mock.Mocker
```

However, it's fine in a `.pyi` file, with `__future__` annotations`, or
if the annotation is in a non-evaluated context, like:

```python
def func():
    item: type[requests_mock.Mocker | httpretty] = requests_mock.Mocker
```

This PR modifies the rule to avoid enforcing in those invalid,
runtime-evaluated contexts.

Closes #6455.
@adamtheturtle
Copy link
Author

Thank you for fixing this @charliermarsh . As it seems you can detect this case, perhaps it would be good to have this case be a ruff error? (though maybe that's more of a job for mypy which also has no problem with it).

durumu pushed a commit to durumu/ruff that referenced this issue Aug 12, 2023
)

## Summary

The use of `|` as a union operator is not always safe, if a type
annotation is evaluated in a runtime context. For example, this code
errors at runtime:

```python
import httpretty
import requests_mock

item: type[requests_mock.Mocker | httpretty] = requests_mock.Mocker
```

However, it's fine in a `.pyi` file, with `__future__` annotations`, or
if the annotation is in a non-evaluated context, like:

```python
def func():
    item: type[requests_mock.Mocker | httpretty] = requests_mock.Mocker
```

This PR modifies the rule to avoid enforcing in those invalid,
runtime-evaluated contexts.

Closes astral-sh#6455.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants