Add pyright config and linting documentation

[tobiasdiez]

This PR provides a minimal configuration for the static typing checker pyright. In #29775 typing for the manifolds package is added. As this is currently the only place where typing is used, it's the only source folder for pyright.

There are also quite a few other code check options. These seem to cover similar rules as pyflakes and pycodestyle. Since these two checker have problems with typings (at least in the versions that they are currently used in the patchbot), it might be worthwhile to investigate if they can be completely replaced by pyright.

Depends on #30408

CC: @mkoeppe @fchapoton @mjungmath @kliem

Component: build

Author: Tobias Diez

Branch/Commit: 7049f39

Reviewer: Matthias Koeppe, Dima Pasechnik

Issue created by migration from https://trac.sagemath.org/ticket/30361

[tobiasdiez]

Commit: 6ab9fe9

[tobiasdiez]

Changed branch from public/manifolds/pyright to public/mainfolds/pyright

[tobiasdiez]

New commits:

6ab9fe9

Add pyright config

[mkoeppe]

comment:3

It would be nice to be able to replicate the style that the current patchbot plugins enforce using these other tools. That would allow developers to form an informed opinion on what tools we should be focusing on...

[tobiasdiez]

comment:4

Pyright is in the first place a static type checker, something that pyflakes does not do (or at least is not designed to do).

Not that this PR only adds the config file, which aids the developer if he chooses to use pyright locally. It doesn't say that it needs to be used, or is integrated in the build workflow. That can be decided later. Given that pyright relies on node and has superior performance over alternatives as myphy, I could imagine that its good to be used in the build workflow and by developers that use VS code. Python-puristic developers propbably prefer a checker that can be installed via pip.

So I would propose to keep this PR as simple as possible, just add the config file for those people that want to use pyright, and add alternatives / integration with the build workflow later.

[mkoeppe]

comment:5

Could you include a comment at the top of this config file with a pointer to documentation or something?

[sagetrac-git]

Changed commit from 6ab9fe9 to 256a9f5

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

256a9f5

Extend configuration

[tobiasdiez]

comment:7

It's not possible to add comments to json files. But from the name of the file it should be rather clear that it belongs to pyright.

I've extended the pyright config a bit. There are no errors (for the manifolds part), but about 200 warnings. Some of them are false positives due to issues of pyright with cython / lazy import (reported as microsoft/pyright#954 and microsoft/pyright#952), but mostly they are valid things that should be fixed (e.g. possibly unbound variables). If you run pyright across the whole sage code, then there are 400 errors and 20k warnings.

I've also fixed one small issue where a getter was used instead of a setter.

[sagetrac-git]

Changed commit from 256a9f5 to dc11c26

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

dc11c26

src/doc/en/developer/tools.rst: Add stub

[mkoeppe]

comment:10

How about adding a little section in the developer's guide?

[sagetrac-git]

Changed commit from dc11c26 to 1f87638

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

a04823f	Add documentation
1f87638	Merge branch 'public/mainfolds/pyright' of git://trac.sagemath.org/sage into public/mainfolds/pyright

[tobiasdiez]

comment:12

Done, anything else to do?

[mkoeppe]

comment:13

Could you move this new text to the section that I created? src/doc/en/developer/tools.rst?

[mkoeppe]

comment:14

Also I was hoping for more hands-on documentation -- show what users need to install & type rather than just sending people to the tool's documentation

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

2a54d7c	Merge branch 'develop' of git://github.com/sagemath/sage into public/mainfolds/pyright
86e21f3	Improve documentation

[sagetrac-git]

Changed commit from 1f87638 to 86e21f3

[tobiasdiez]

comment:16

Ohh, sorry, I've overlooked that you've already created a new file for it. Thanks!
I've now expanded the documentation.

[mkoeppe]

comment:17

Please take a look at #30410 as a possible interface to be advertised in the documentation.

[mkoeppe]

Dependencies: #30408, #30410

[tobiasdiez]

comment:19

I think it's a good idea to have a consistent interface for running linters and other checks . Thus I welcome the idea of #30410. However, this is rather unrelated to this ticket so I reversed the dependencies.

[mkoeppe]

comment:35

The only way to validate it is to build the documentation using make doc-html...

It would actually be great to have a quick validation which we could plug into tox

[mkoeppe]

comment:36

patchbot is green, positive review

[dimpase]

comment:37

Some types info has been added in a new code in recent tickets by gh-Ivo-Maffei, e.g. in #30337

Does this mean that pyrightconfig.json has to be modified accordingly?

[tobiasdiez]

comment:38

Since pyright is currently not used in the patchbot (or something similar), the configuration file is only important for people that use pyright locally in their development. I would thus suggest that as soon as a bit of typing information has been added in a certain package, than this package should be added to the pyright include statement.

The long term goal should be to set pyrights rules to strict and have all of sage's code tested for each code contribution. I'm not sure what's the most practical way to achieve this.

[vbraun]

comment:39

PDF docs don't build

[dimpase]

comment:40

The error is

[docpdf] ! You can't use `macro parameter character #' in math mode.
[docpdf] l.7363 ...n <https://github.com/microsoft/pyright#
[docpdf]                                                   installation>\) for details.
[docpdf] ? 
[docpdf] ! Emergency stop.

[tobiasdiez]

comment:41

Ok, but what is wrong with this link? Should I remove the #installation part?

[sagetrac-git]

Changed commit from 4159c3a to 7049f39

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

7049f39

add missing '_'s

[dimpase]

comment:43

a typical issue of weird rst syntax (missing _)

cf https://docutils.sourceforge.io/docs/user/rst/quickref.html#hyperlink-targets

[tobiasdiez]

comment:44

Thanks!

[dimpase]

comment:45

I'm still checking that the pdf docs build (it requires a more complete install of TexLive than I have, and so takes a while)

[dimpase]

Changed reviewer from Matthias Koeppe to Matthias Koeppe, Dima Pasechnik

[dimpase]

comment:46

OK, alles gut/goed, een kleine taaloefening voor \usepackage{babel}

[tobiasdiez]

comment:47

Is there anything that needs to be done from my side for this to be merged?

[fchapoton]

comment:48

patience, please

[vbraun]

Changed branch from public/mainfolds/pyright to 7049f39