pyre-check icon indicating copy to clipboard operation
pyre-check copied to clipboard

Pyre failing in python 3.8 and 3.9 due to new syntax

Open henrylhtsang opened this issue 10 months ago • 9 comments

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 avatar Apr 23 '24 02:04 henrylhtsang

cc @MaggieMoss

henrylhtsang avatar Apr 23 '24 02:04 henrylhtsang

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.

migeed-z avatar Apr 23 '24 18:04 migeed-z

@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

henrylhtsang avatar Apr 23 '24 19:04 henrylhtsang

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

migeed-z avatar Apr 23 '24 20:04 migeed-z

@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.

henrylhtsang avatar Apr 23 '24 22:04 henrylhtsang

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

migeed-z avatar Apr 23 '24 23:04 migeed-z

@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

henrylhtsang avatar Apr 24 '24 00:04 henrylhtsang

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.

migeed-z avatar Apr 24 '24 01:04 migeed-z

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

  • #832

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.

cclauss avatar Apr 24 '24 16:04 cclauss

Closing the loop on this: I backed all these out to only use Union.

We cannot make use of from __future__ import annotations because this code is using the types at runtime (via dataclasses_json.

I've increased the number of tests we run in github CI to make this kind of error less likely (we have to rely on github CI because internally we only use 3.10, so we tend to not notice 3.8/3.9 issues right away)

stroxler avatar May 29 '24 23:05 stroxler