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

[Feature] Pyright strict mode compatibility #1532

Closed
jcheng5 opened this issue Aug 25, 2022 · 2 comments · Fixed by #1536
Closed

[Feature] Pyright strict mode compatibility #1532

jcheng5 opened this issue Aug 25, 2022 · 2 comments · Fixed by #1536
Labels

Comments

@jcheng5
Copy link
Contributor

jcheng5 commented Aug 25, 2022

Hi! I'm trying to use playwright with my codebase, which uses pyright in strict mode. I'm finding that assertions in the sync API make pyright unhappy, because NoneType turns into Unknown:

# pyright: strict

from playwright.sync_api import Page, expect


def test_app(page: Page):
    page.goto("http://127.0.0.1:8000/")
    plot = page.locator("#plot")
    expect(plot).to_have_class("text-center")
$ pyright
No configuration file found.
No pyproject.toml file found.
stubPath /Users/jcheng/Development/jcheng5/nonetype-test/typings is not a valid directory.
Assuming Python platform Darwin
Searching for source files
Found 1 source file
pyright 1.1.268
/Users/jcheng/Development/jcheng5/nonetype-test/test_playwright.py
  /Users/jcheng/Development/jcheng5/nonetype-test/test_playwright.py:9:5 - error: Type of "to_have_class" is partially unknown
    Type of "to_have_class" is "(expected: List[Pattern[str] | str] | Pattern[str] | str, *, timeout: float | None = None) -> Unknown" (reportUnknownMemberType)
1 error, 0 warnings, 0 informations
Completed in 0.634sec

If I replace all the NoneType annotations in playwright/sync_api/_generated.py with None, the error goes away.

(I just want to add, this is the only problem I've run into with playwright so far--in every other respect it's been a pleasure to use!)

@rwoll rwoll added the triaging label Aug 30, 2022
@rwoll
Copy link
Member

rwoll commented Aug 30, 2022

I'll raise with the team to better understand None v. NoneType choice here.

@jcheng5
Copy link
Contributor Author

jcheng5 commented Sep 1, 2022

I've researched this as much as I can and came to the below conclusions, which I'm spelling out here on the off chance that it's helpful for your team. I also created PR #1536 in an effort to make this fix as low effort as possible.

NoneType is the type of the None object, but prior to Python 3.10 it was only internal. I am guessing NoneType ended up in the playwright APIs only because of how the Python annotation object represents itself as a string:

>>> def foo() -> None: ...
>>> str(typing.get_type_hints(foo)["return"])
"<class 'NoneType'>"

And this line in process_type basically grabs whatever's in <class '(.+?)'>.

However, it's definitely not correct to actually use NoneType in your source code's type annotations; you're supposed to just use None instead. The playwright generated API files have to introduce a type alias for it (e.g. here).

The mypy docs have this to say:

The Python interpreter internally uses the name NoneType for the type of None, but None is always used in type annotations. The latter is shorter and reads better. (NoneType is available as types.NoneType on Python 3.10+, but is not exposed at all on earlier versions of Python.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants