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

Pyre failing in python 3.8 and 3.9 due to new syntax #827

Open
henrylhtsang opened this issue Apr 23, 2024 · 9 comments
Open

Pyre failing in python 3.8 and 3.9 due to new syntax #827

henrylhtsang opened this issue Apr 23, 2024 · 9 comments

Comments

@henrylhtsang
Copy link

Pyre Bug

Bug description

When using python 3.8 and 3.9 with pyre, getting the following error due to
code: Optional[int | str] = None
https://github.com/facebook/pyre-check/blob/main/client/language_server/protocol.py#L365

Reproduction steps
Enter steps to reproduce the behavior.
Follow https://github.com/pytorch/torchrec/blob/main/.github/workflows/pyre.yml to run pyre check with "version": "0.0.101711451648"

Expected behavior
It should run.

The syntax is only created in python 3.10 and after.
https://stackoverflow.com/questions/76712720/typeerror-unsupported-operand-types-for-type-and-nonetype

Logs

Traceback (most recent call last):
  File "/usr/share/miniconda/envs/test/bin/pyre", line 5, in <module>
    from pyre_check.client.pyre import main
  File "/usr/share/miniconda/envs/test/lib/python3.[8](https://github.com/pytorch/torchrec/actions/runs/8793083113/job/24130384280#step:5:9)/site-packages/pyre_check/client/pyre.py", line 31, in <module>
    from . import (
  File "/usr/share/miniconda/envs/test/lib/python3.8/site-packages/pyre_check/client/commands/__init__.py", line 16, in <module>
    from . import (  # noqa F401
  File "/usr/share/miniconda/envs/test/lib/python3.8/site-packages/pyre_check/client/commands/analyze.py", line 30, in <module>
    from . import commands, start, validate_models
  File "/usr/share/miniconda/envs/test/lib/python3.8/site-packages/pyre_check/client/commands/start.py", line 51, in <module>
    from . import commands, server_event, stop
  File "/usr/share/miniconda/envs/test/lib/python3.8/site-packages/pyre_check/client/commands/server_event.py", line 22, in <module>
    from ..language_server import connections
  File "/usr/share/miniconda/envs/test/lib/python3.8/site-packages/pyre_check/client/language_server/__init__.py", line [13](https://github.com/pytorch/torchrec/actions/runs/8793083113/job/24130384280#step:5:14), in <module>
    from . import (  # noqa F401
  File "/usr/share/miniconda/envs/test/lib/python3.8/site-packages/pyre_check/client/language_server/code_navigation_request.py", line 22, in <module>
    from . import daemon_connection, protocol as lsp
  File "/usr/share/miniconda/envs/test/lib/python3.8/site-packages/pyre_check/client/language_server/protocol.py", line 361, in <module>
    class Diagnostic(json_mixins.CamlCaseAndExcludeJsonMixin):
  File "/usr/share/miniconda/envs/test/lib/python3.8/site-packages/pyre_check/client/language_server/protocol.py", line 365, in Diagnostic
    code: Optional[int | str] = None
TypeError: unsupported operand type(s) for |: 'type' and 'type'
@henrylhtsang
Copy link
Author

cc @MaggieMoss

@migeed-z
Copy link

Does the following work for you?

from typing import *

x: Optional[int | str] = None

It does on the latest version of Pyre. If this doesn't work, we may need to do an upgrade.

@henrylhtsang
Copy link
Author

@migeed-z not suer if I understand correctly, I thought it got an error since int | str is a new syntax for python 3.10 only? And that part code: Optional[int | str] = None is in pyre code

@migeed-z
Copy link

To verify if the error is due to unsupported syntax, could you doublecheck if the code above type check for you?

@henrylhtsang
Copy link
Author

@migeed-z interesting, I tested in https://github.com/pytorch/torchrec/actions/runs/8807458504/job/24174554261

pyre is python 3.8 with "version": "0.0.101703592829", but does not see an error.

@migeed-z
Copy link

Did you test that one small repro or the entire code?

@henrylhtsang
Copy link
Author

@migeed-z I added that code snippet to a single unit test, and then run the pyre github action with it.

This one:

from typing import *

x: Optional[int | str] = None

@migeed-z
Copy link

I see. Thanks for the information. Are we able to run on the same version of the original code though? because the error message you have originally suggests the syntax is not supported, so I wonder if there is a variation between both versions or if your original code has a different issue.

@cclauss
Copy link
Contributor

cclauss commented Apr 24, 2024

On Python < 3.10 the source file must start with from __future__ import annotations to be able to use PEP 604 syntax.

On Python < 3.10, there should be two tests:

  1. A source file that starts with from __future__ import annotations should pass if it uses PEP 604 syntax.
  2. A source file that starts without from __future__ import annotations should fail if it uses PEP 604 syntax.

facebook-github-bot pushed a commit that referenced this issue Apr 25, 2024
Summary:
**Pre-submission checklist**
- [ ] I've ran the linters locally and fixed lint errors related to the files I modified in this PR. You can install the linters by running `pip install -r requirements-dev.txt && pre-commit install`
- [ ] `pre-commit run`

Related to:
* #827
* #821
* #816

PEP 604 – Allow writing union types as `X | Y` -- https://peps.python.org/pep-0604
```
   File "/home/runner/work/pyre-check/pyre-check/client/language_server/protocol.py", line 365, in Diagnostic
    code: Optional[int | str] = None
TypeError: unsupported operand type(s) for |: 'type' and 'type'
```
On Python < 3.10 the tests are failing on PEP 604 type annotations which will be enforced by `ruff rule UP007`.
* https://docs.astral.sh/ruff/rules/non-pep604-annotation/#pyupgrade-up

On Python < 3.10 the source file ***must*** start with `from __future__ import annotations` to be able to use PEP 604 syntax.

Pull Request resolved: #832

Reviewed By: stroxler

Differential Revision: D56535593

Pulled By: migeed-z

fbshipit-source-id: 27b470c5cd16752615a16f1665a8366891e4bcc3
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 a pull request may close this issue.

3 participants