Support mypy #699
Comments
|
The biggest reason as to why this project only supports pyright is that mypy often takes significantly longer than pyright to implement any new typing features. In the time of adding pyright to the project, mypy still didn't even have support for Since then, I haven't really been keeping up to date with mypy's development, and it's entirely possible that they've got better at keeping up though. However if that's not the case, I'm not sure I'd like to bring it in, as it could potentially prevent us from being able to use some nice new typing features. That said, if things are better now, I'm not necessarily against adding mypy alongside with pyright. I don't think we would make any use of mypyc though, and I'd like to have a better reason to make this change than just:
Contributors are expected to conform to the project's style guidelines, and to set up their development environment for the project they're working on accordingly to those guidelines, regardless of what they prefer. (Hint: You don't need to have pyright in your editor if you really don't want to, just make sure to run pre-commit to ensure that pyright will pass, and not to introduce any mypy-specific stuff. Although adding pyright to your editor would probably result in a better experience. Most editors should be configurable in a way that lets you disable/enable various linters/type-checkers on a per-project basis, so it doesn't have to change your workflow on other projects.) Even if mypy support would be added though, it won't mean removal of pyright, we would just be running both, so you'll probably want to set up pyright for mcstatus anyway. I would like to ask all other contributors reading this to give their opinion on adding mypy, either by commenting, or at least by adding a 👍 or 👎 reaction to the issue, as it is a fairly significant change, and makes mypy knowledge yet another pre-requisite, alongside pyright and all of our other linters. |
As I can understand, you get errors when using this project as library in another project? If so, could you please provide what errors you get? I used this library in projects powered by mypy a lot and didn't get any problems.
I also ran this small script using mypyc just fine without any changes to the project code. # mypyc_test.py
import time
import mcstatus
t0 = time.time()
server = mcstatus.JavaServer.lookup("play.hypixel.net")
status = server.status()
print(status.motd)
print(time.time() - t0)~/dev/mcstatus master ❯ python mypyc_test.py
Motd(parsed=[' ', <MinecraftColor.GREEN: 'a'>, 'Hypixel Network ', <MinecraftColor.RED: 'c'>, '[1.8-1.20]\n ', <MinecraftColor.DARK_AQUA: '3'>, '', <Formatting.BOLD: 'l'>, 'SKYBLOCK 0.19.9 ', <MinecraftColor.GRAY: '7'>, '| ', <MinecraftColor.AQUA: 'b'>, '', <Formatting.BOLD: 'l'>, 'BEDWARS v1.9'], raw=' §aHypixel Network §c[1.8-1.20]\n §3§lSKYBLOCK 0.19.9 §7| §b§lBEDWARS v1.9', bedrock=False)
0.45096421241760254
~/dev/mcstatus master ❯ mypyc mypyc_test.py
running build_ext
building 'mypyc_test' extension
creating build/temp.linux-x86_64-cpython-312
creating build/temp.linux-x86_64-cpython-312/build
gcc -fno-strict-overflow -Wsign-compare -DDYNAMIC_ANNOTATIONS_ENABLED=1 -DNDEBUG -fcf-protection -fexceptions -fcf-protection -fexceptions -fcf-protection -fexceptions -fPIC -I/home/perchun/.cache/pypoetry/virtualenvs/mcstatus-Owe_Dc5Q-py3.12/lib64/python3.12/site-packages/mypyc/lib-rt -I/home/perchun/.cache/pypoetry/virtualenvs/mcstatus-Owe_Dc5Q-py3.12/include -I/usr/include/python3.12 -c build/__native.c -o build/temp.linux-x86_64-cpython-312/build/__native.o -O3 -g1 -Werror -Wno-unused-function -Wno-unused-label -Wno-unreachable-code -Wno-unused-variable -Wno-unused-command-line-argument -Wno-unknown-warning-option -Wno-unused-but-set-variable -Wno-ignored-optimization-argument -Wno-cpp
creating build/lib.linux-x86_64-cpython-312
gcc -shared build/temp.linux-x86_64-cpython-312/build/__native.o -L/usr/lib64 -o build/lib.linux-x86_64-cpython-312/mypyc_test.cpython-312-x86_64-linux-gnu.so
copying build/lib.linux-x86_64-cpython-312/mypyc_test.cpython-312-x86_64-linux-gnu.so ->
~/dev/mcstatus master ❯ python -c "import mypyc_test"
Motd(parsed=[' ', <MinecraftColor.GREEN: 'a'>, 'Hypixel Network ', <MinecraftColor.RED: 'c'>, '[1.8-1.20]\n ', <MinecraftColor.DARK_AQUA: '3'>, '', <Formatting.BOLD: 'l'>, 'SKYBLOCK 0.19.9 ', <MinecraftColor.GRAY: '7'>, '| ', <MinecraftColor.AQUA: 'b'>, '', <Formatting.BOLD: 'l'>, 'BEDWARS v1.9'], raw=' §aHypixel Network §c[1.8-1.20]\n §3§lSKYBLOCK 0.19.9 §7| §b§lBEDWARS v1.9', bedrock=False)
0.3873469829559326And to be fare:
|
Even if they are so, I would expect conflicts between those too (as mypy has warnings about unused type ignores, and pyright has more checks than mypy which may be false-positive/inaccurate sometimes) So to summarize, I don't see any sense in keeping both, as they would likely conflict. If you get errors in your project, where mcstatus is used properly as library installed through pip (not like cloned with git in the same folder) - it is probably a bug. |
Those warnings can be disabled, also pyright has |
|
Oh and here are errors raised by mypy now. Some of them are covered in newer version of pyright (I will do fixes later, but if someone would want to take it - I don't mind) and some of them are mypy-specific false-positives. Although there are errors, that weren't catched by pyright. |
|
Many of the errors that weren't checked by pyright is because we didn't enable some optional extra checks that pyright has, but yeah nevertheless, mypy is generally more strict than pyright. |
|
There's a really nice document showing the differences between the two type-checkers here: https://github.com/microsoft/pyright/blob/main/docs/mypy-comparison.md. Could be useful when considering whether we'd like to add it. |
|
Running both pyright and mypy would increase our maintenance burden for false positives. So, that leaves the option of selecting one over the other. Pyright has proven helpful in improving mcstatus and detecting some real issues. I would want to see proof of mistakes that mypy is catching that pyright cannot catch before considering a switch. Reading through that mypy output above shows me that mypy is catching a lot of false positives that we already handled with pyright. Until a worthwhile proof appears, pyright is handling the job. |
Officially supporting type checking with mypy as well would mean I don't get type errors in my project just because I don't use pyright and opens the door to using mypyc to compile the project into a C extension, which in some cases can make things run faster.
The text was updated successfully, but these errors were encountered: