[regression] Mypy fails with shiny==0.9.0

[pstorozenko]

Hi,
I've found the following regression.

Problem

With mypy==1.9.0 and shiny==0.9.0 and

import shiny

in app.py.

mypy app.py raises

➜ mypy app.py
/Users/pasza/Library/Caches/pypoetry/virtualenvs/test-shiny09-qk-unJu3-py3.12/lib/python3.12/site-packages/shiny/render/_data_frame_utils/_selection.py:155: error: invalid syntax  [syntax]
Found 1 error in 1 file (errors prevented further checking)

This doesn't happen with shiny==0.8.1, I get the expected Success: no issues found in 1 source file.

Reason

The reason is the commented-out code in the source code 🙈
In _selection.py, lines 154-155 are:

# class CellSelectionAll(TypedDict):
#     type: Literal["all"]

mypy (I guess) interprets # type: as some kind of type hint for the thing in front of the # sign. Or something like that, just like # type: ignore. Or maybe Literal["all"] is just not proper syntax for # type:...

Potential fix

Remove unused code, or mangle the commented code to not make it clash with mypy.

After thought

Maybe this check could've/should've been added to CICD?

Best,
Pasza

[tool.poetry]
name = "test-shiny09"
version = "0.1.0"
description = ""
authors = ["Piotr Pasza Storozenko <piotr@appsilon.com>"]
readme = "README.md"

[tool.poetry.dependencies]
python = "^3.12"
shiny = "0.9.0"
mypy = "^1.9.0"


[build-system]
requires = ["poetry-core"]
build-backend = "poetry.core.masonry.api"

[wch]

I made a quick fix in 94d84c3.

We currently use pyright in strict mode for type checking this project. In my opinion, pyright works much better in my than mypy.

In theory, type checking with mypy in CI would be good, to avoid problems like the one that you ran into, where a project that that imports shiny makes use of mypy to do type checking. However, I don't think that using mypy in addition to pyright is practical for Shiny, because there are differences in the behavior of mypy and pyright, as documented here:
https://github.com/microsoft/pyright/blob/main/docs/mypy-comparison.md

On the Shiny code base, pyright reports zero type errors, but mypy reports 180 errors. I don't think getting it to pass both systems would not make the code meaningfully better.

The change I made in 94d84c3 fixed the syntax error, but mypy still thinks there are type errors. Hopefully fixing just the syntax error is enough to satisfy the use case you have run into.

[pstorozenko]

Thanks for the fix.

Hmm, I personally don't personally like the fact that pyright has js dependencies and always preferred mypy for CI/CD (I use pyright in vs code), but it's good to know shiny has pyright-first support when it comes to type checking, thanks.

As you mentioned, making shiny internal code base pass both systems doesn't make sense to me neither, but making sure that mypy users can just import shiny without problems in their code base could be beneficial.

[wch]

It would be nice if it were as simple as having an app.py with import shiny in it, like in your case. However, there are a number submodules which are not automatically loaded this way, and so they would not be "tested" for syntax errors under mypy.

If it were possible to run mypy on the whole shiny codebase and report only mypy syntax errors, that is probably what we would want. However, I took a quick look at the mypy documentation and did not see a way to check only for mypy syntax errors. If you know of a way to do that, please let us know.

[gabrielecalvo]

The same problem re-appeared on shiny==1.0.0

Looks like some work in the mypy codebase has been done to try ignore the error above, but it's currently still not fixed.

Currently one way to check for this specific issue would be to run: grep -R '^# \+type:' shiny

[wch]

@gabrielecalvo Do you know which lines in the Shiny code base cause the problems with mypy?

[gabrielecalvo]

yep, it's the same one as before actually..

$ mypy app.py

.venv\Lib\site-packages\shiny\render\_data_frame_utils\_selection.py:154: error: invalid syntax  [syntax]
Found 1 error in 1 file (errors prevented further checking)

(using current latest versions mypy==1.11.1 and shiny==1.0.0)