Opened 22 months ago

Closed 7 weeks ago

#30411 closed enhancement (fixed)

src/tox.ini: Add environment pyright

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-9.6
Component: doctest framework Keywords:
Cc: gh-tobiasdiez, chapoton Merged in:
Authors: Matthias Koeppe Reviewers: Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: a2e56de (Commits, GitHub, GitLab) Commit: a2e56de453fdc6dd3ea90b8e8b9d5064a289c132
Dependencies: Stopgaps:

Status badges

Change History (36)

comment:1 Changed 22 months ago by mkoeppe

  • Description modified (diff)

comment:2 Changed 21 months ago by mkoeppe

  • Dependencies set to #30246

comment:3 Changed 21 months ago by gh-tobiasdiez

  • Description modified (diff)

comment:4 Changed 21 months ago by mkoeppe

  • Description modified (diff)

comment:5 Changed 21 months ago by mkoeppe

  • Branch set to u/mkoeppe/src_tox_ini__add_environment_pyright

comment:6 Changed 21 months ago by mkoeppe

  • Commit set to a717cd2c592fbf84afeef12435dcdbbc288f1ba5
  • Dependencies changed from #30246 to #30246, #30361, #30467

Here's a version that succeeds in installing pyright (in the toxenv). But then testing does not work because it also needs to import sage.


Last 10 new commits:

939e9e0src/tox.ini: Better default arg testenv:pycodestyle
583bde5Merge branch 't/30408/public/build/pycodestyleConfig' into t/30410/command__sage__tox_
8574448src/tox.ini: Add coverage, startuptime; refactor, add descriptions
eba708btox.ini: Delegate doctest, coverage, startuptime, pycodestyle to src/tox.ini
df7ef3fMerge branch 't/30410/command__sage__tox_' into t/30467/src_tox_ini__check_patchbot_plugin_patterns
7e9d12dsrc/tox.ini: New environment relint
4a3a33aAdd comment
b0ad03esrc/bin/sage: Show tox environment list in 'sage -advanced'
23753a9Merge branch 't/30467/src_tox_ini__check_patchbot_plugin_patterns' into t/30411/src_tox_ini__add_environment_pyright
a717cd2WIP: src/.tox.ini: Add testenv:pyright

comment:7 Changed 19 months ago by mkoeppe

  • Milestone changed from sage-9.2 to sage-9.3

comment:8 Changed 16 months ago by mkoeppe

  • Milestone changed from sage-9.3 to sage-9.4

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.

comment:9 Changed 10 months ago by mkoeppe

  • Milestone changed from sage-9.4 to sage-9.5

comment:10 Changed 10 months ago by git

  • Commit changed from a717cd2c592fbf84afeef12435dcdbbc288f1ba5 to 93eb7f48f131ec0ddbd0032e6d8b1b7c82a49b68

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

93eb7f4WIP: src/.tox.ini: Add testenv:pyright

comment:11 Changed 10 months ago by mkoeppe

rebased

comment:12 Changed 10 months ago by mkoeppe

  • Dependencies #30246, #30361, #30467 deleted

comment:13 Changed 6 months ago by mkoeppe

  • Milestone changed from sage-9.5 to sage-9.6

comment:14 Changed 2 months ago by mkoeppe

This can now be simplified using https://pypi.org/project/pyright/

comment:15 Changed 2 months ago by git

  • Commit changed from 93eb7f48f131ec0ddbd0032e6d8b1b7c82a49b68 to 70bf65238f3185bdba09a709023f25d8d25db21c

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

a3d301cWIP: src/.tox.ini: Add testenv:pyright
70bf652src/tox.ini (pyright): Use pypi package pyright

comment:16 Changed 2 months ago by git

  • Commit changed from 70bf65238f3185bdba09a709023f25d8d25db21c to 71649fe5a230fd1066424ac228628930a5e4dca8

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

71649fetox.ini (pyright): Fixup

comment:17 Changed 2 months ago by mkoeppe

  • Description modified (diff)

comment:18 Changed 2 months ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Status changed from new to needs_review

comment:19 Changed 2 months ago by git

  • Commit changed from 71649fe5a230fd1066424ac228628930a5e4dca8 to 2f7d2959f98a8ae4c66f6115f3ebc143850a3b30

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

2707251WIP: src/.tox.ini: Add testenv:pyright
1f39c9bsrc/tox.ini (pyright): Use pypi package pyright
f9c25ebtox.ini (pyright): Fixup
79d7c38Readd pyright and fix small errors
2f7d295Demote missing imports to warning for now as well

comment:20 Changed 2 months ago by mkoeppe

  • Dependencies set to #33456

comment:21 Changed 2 months ago by git

  • Commit changed from 2f7d2959f98a8ae4c66f6115f3ebc143850a3b30 to e92ad80a99f743f70c6d721b282644ab6c9e7e6a

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

aa4a951Readd pyright and fix small errors
acad07cDemote missing imports to warning for now as well
e92ad80Merge #33456

comment:22 Changed 2 months ago by mkoeppe

Locally (macOS), ./sage -tox -e pyright (in a build with ./configure --enable-editable) ends with

FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory

comment:23 Changed 2 months ago by git

  • Commit changed from e92ad80a99f743f70c6d721b282644ab6c9e7e6a to 3277d2924805c5090ed19b32b2fe4a56a70352f0

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

d20fcd5WIP: src/.tox.ini: Add testenv:pyright
f837b3asrc/tox.ini (pyright): Use pypi package pyright
4ae0668tox.ini (pyright): Fixup
3277d29src/tox.ini (pyright): Run it in its own toxenv; default to running on sage.manifolds only

comment:24 Changed 2 months ago by mkoeppe

  • Cc chapoton added
  • Dependencies #33456 deleted

comment:25 Changed 2 months ago by mkoeppe

  • Description modified (diff)

sage -tox -e pyright now works well for me.

However, pyright outputs lots of nonsensical warnings, so I am not adding it to the list of tox environments that will be tested by default if one just uses sage -tox.

comment:26 Changed 2 months ago by git

  • Commit changed from 3277d2924805c5090ed19b32b2fe4a56a70352f0 to ef0eeb7897716b786073b54696234720cad11867

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

ef0eeb7src/doc/en/developer/tools.rst: Update section on pyright

comment:27 Changed 2 months ago by mkoeppe

Ready for review

comment:28 Changed 2 months ago by gh-tobiasdiez

  • (for manual use or use with VS Code:) npm install -g pyright, see documentation <https://github.com/microsoft/pyright#installation> for details.

You don't have to install pyright manually for use in vscode, its done via the pylance extension (providing such a seamless integration with vs code was one of the reasons why pyright is written in ts).

comment:29 Changed 2 months ago by git

  • Commit changed from ef0eeb7897716b786073b54696234720cad11867 to ec4588800e2fd278266b447fff095e36a2aa1b87

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

ec45888src/doc/en/developer/tools.rst: no need to install pyright manually for use in vscode

comment:30 Changed 2 months ago by gh-tobiasdiez

Thanks for the update.

With the current config, tox -e pyright still uses the venv symlink to resolve imports etc. Wouldn't it be more in the spirit of tox to install sagelib in the tox env and use that as a reference for imports?

For example, without a properly setup venv, tox -e pyright for example complains

  /workspace/sagetrac-mirror/src/sage/manifolds/subsets/pullback.py:302:22 - warning: Import "ppl" could not be resolved (reportMissingImports)
  /workspace/sagetrac-mirror/src/sage/manifolds/subsets/pullback.py:827:26 - warning: Import "ppl" could not be resolved (reportMissingImports)

These are not displayed if venv is setup correctly.

Also, why do you need SAGE_LOCAL for pyright/in the tox env?

comment:31 Changed 2 months ago by mkoeppe

It's likely that I am confused about what pyright does with venvs. This is coming back to https://trac.sagemath.org/ticket/33456#comment:10 - the current configuration in pyrightconfig.json makes no sense to me when I read the documentation of pyright.

comment:32 Changed 2 months ago by gh-tobiasdiez

That config is also highly confusing, and took me quite some time to understand.

venvPath is the path where all venv's are supposed to be, and venv is then a path relative to this path selecting a certain venv. So with venvPath = /home/venvs and venv = ./a, the venv is supposed to be in /home/venvs/a.

The code explains this better then the docs: https://github.com/microsoft/pyright/blob/fa4194fd8b0e6d9fb970468e50a974e0d4eeee5c/packages/pyright-internal/src/analyzer/pythonPathUtils.ts#L72-L73

So I guess you only have to set venvPath in the cli to the tox venv, and everything should work.

comment:33 Changed 8 weeks ago by git

  • Commit changed from ec4588800e2fd278266b447fff095e36a2aa1b87 to a2e56de453fdc6dd3ea90b8e8b9d5064a289c132

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

42b4c24WIP: src/.tox.ini: Add testenv:pyright
7b64334src/tox.ini (pyright): Use pypi package pyright
db7dba6tox.ini (pyright): Fixup
a20e6b5src/tox.ini (pyright): Run it in its own toxenv; default to running on sage.manifolds only
0ec81a5src/doc/en/developer/tools.rst: Update section on pyright
ebdd23dsrc/doc/en/developer/tools.rst: no need to install pyright manually for use in vscode
a2e56desrc/tox.ini (pyright): Pin pyright version

comment:34 Changed 8 weeks ago by dimpase

  • Reviewers set to Dima Pasechnik
  • Status changed from needs_review to positive_review

lgtm

comment:35 Changed 8 weeks ago by mkoeppe

Thanks

comment:36 Changed 7 weeks ago by vbraun

  • Branch changed from u/mkoeppe/src_tox_ini__add_environment_pyright to a2e56de453fdc6dd3ea90b8e8b9d5064a289c132
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.