-
Notifications
You must be signed in to change notification settings - Fork 815
[checks] Implement Python checks and fix some violations #9146
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| name: check-python | ||
|
|
||
| on: | ||
| pull_request: | ||
| paths: | ||
| - "tools/**.py" | ||
| - "pyrightconfig.json" | ||
| - ".github/workflows/check-python.yml" | ||
| push: | ||
| paths: | ||
| - "tools/**.py" | ||
| - "pyrightconfig.json" | ||
| - ".github/workflows/check-python.yml" | ||
|
|
||
| jobs: | ||
| pyright: | ||
| name: Pyright type check | ||
| runs-on: ubuntu-24.04 | ||
|
|
||
| steps: | ||
| - name: checkout | ||
| uses: actions/checkout@v6 | ||
|
|
||
| - name: setup Node | ||
| uses: actions/setup-node@v6 | ||
| with: | ||
| node-version: "20" | ||
|
|
||
| - name: install pyright | ||
| run: npm install -g pyright | ||
|
|
||
| - name: run pyright | ||
| run: pyright | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,3 +22,4 @@ tools/sections | |
| *.synctex.gz | ||
| *.synctex* | ||
| .check.stamp | ||
| __pycache__ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While it won't matter until we have multiple Python modules, you might want to also ignore |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| { | ||
| "include": [ | ||
| "tools/*.py" | ||
| ], | ||
| "typeCheckingMode": "strict", | ||
| "reportMissingTypeStubs": false, | ||
| "reportUnknownMemberType": false, | ||
| "reportUnknownArgumentType": false, | ||
| "reportUnknownVariableType": false, | ||
| "reportUnknownParameterType": false, | ||
| "reportUnusedImport": true, | ||
| "reportUnusedVariable": true, | ||
| "pythonVersion": "3.11" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why 3.11 specifically? That version of Python is EoL in about a year. For the CI we use Ubuntu 24.04. This ships with Python 3.12 - the current Python release is 3.14
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just blindly copied and pasted this from another project, so no particular reason. |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| %NOCHECKBEGIN | ||
| \documentclass[9pt]{standalone} | ||
|
|
||
| \usepackage{fontspec} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| %NOCHECKBEGIN | ||
| \documentclass[9pt]{standalone} | ||
|
|
||
| \usepackage{fontspec} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| %NOCHECKBEGIN | ||
| \documentclass[9pt]{standalone} | ||
|
|
||
| \usepackage{fontspec} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| %NOCHECKBEGIN | ||
| \documentclass[9pt]{standalone} | ||
|
|
||
| \usepackage{fontspec} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| %NOCHECKBEGIN | ||
| \documentclass[9pt]{standalone} | ||
|
|
||
| \usepackage{fontspec} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| %NOCHECKBEGIN | ||
| \documentclass[9pt]{standalone} | ||
|
|
||
| \usepackage{fontspec} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| %!TEX root = std.tex | ||
| %NOCHECKBEGIN(lib-*) | ||
| \rSec0[basic]{Basics} | ||
|
|
||
| \gramSec[gram.basic]{Basics} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| %!TEX root = std.tex | ||
| %NOCHECKBEGIN(lib-*) | ||
| \rSec0[class]{Classes}% | ||
| \indextext{class|(} | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| %!TEX root = std.tex | ||
| %NOCHECKBEGIN(lib-*) | ||
| \rSec0[dcl]{Declarations}% | ||
| \indextext{declaration|(} | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| %!TEX root = std.tex | ||
| %NOCHECKBEGIN(lib-*) | ||
| \rSec0[except]{Exception handling}% | ||
| \indextext{exception handling|(} | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| %!TEX root = std.tex | ||
| %NOCHECKBEGIN(lib-*) | ||
| \rSec0[expr]{Expressions} | ||
|
|
||
| \gramSec[gram.expr]{Expressions} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| %NOCHECKBEGIN(lib-*) | ||
| \infannex{gram}{Grammar summary} | ||
|
|
||
| \rSec1[gram.general]{General} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| %!TEX root = std.tex | ||
| %NOCHECKBEGIN(lib-*) | ||
| \infannex{implimits}{Implementation quantities} | ||
|
|
||
| \pnum | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,6 @@ | ||
| %!TEX root = std.tex | ||
| %NOCHECKBEGIN | ||
| %NOCHECKEND(text-*) | ||
|
|
||
| % Definitions and redefinitions of special commands | ||
| % | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| %!TEX root = std.tex | ||
| %NOCHECKBEGIN(lib-*) | ||
| \rSec0[module]{Modules}% | ||
|
|
||
| \gramSec[gram.module]{Modules} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| %!TEX root = std.tex | ||
| %NOCHECKBEGIN(lib-ranges-*) | ||
| \rSec0[numerics]{Numerics library} | ||
|
|
||
| \rSec1[numerics.general]{General} | ||
|
|
@@ -17724,6 +17725,7 @@ | |
|
|
||
| \pnum | ||
| \constraints | ||
| %NOCHECKBEGIN(base-tcode-exposid) | ||
| Every type in the parameter pack \tcode{Flags} is one of \tcode{\exposid{convert-flag}}, | ||
| \tcode{\exposid{aligned-flag}}, or \tcode{\exposid{over\-aligned-\brk{}flag}<N>}. | ||
|
Comment on lines
+17728
to
17730
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can just remove the \tcode wrapping around \exposid here?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, we could. It would be something like 10 separate fixes. I think I'll just do a surgical fix in a separate PR for these cleanups so that this PR doesn't have to clean up any existing sources. The problem with the existing check is that the regex pattern doesn't work on hyphenated identifiers. |
||
|
|
||
|
|
@@ -19889,6 +19891,7 @@ | |
| where \tcode{\exposid{simd-select-impl}} is found by argument-dependent | ||
| lookup\iref{basic.lookup.argdep} contrary to \ref{contents}. | ||
| \end{itemdescr} | ||
| %NOCHECKEND(base-tcode-exposid) | ||
|
|
||
| \rSec3[simd.math]{Mathematical functions} | ||
|
|
||
|
|
@@ -20946,7 +20949,8 @@ | |
| \tcode{basic_mask<Bytes, Abi>} is trivially copyable. | ||
|
|
||
| \pnum | ||
| \recommended Implementations should support implicit conversions between | ||
| \recommended | ||
| Implementations should support implicit conversions between | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please move all specific fixes to a separate commit. |
||
| specializations of \tcode{basic_mask} and appropriate \impldef{conversions | ||
| of \tcode{basic_mask} from/to implementation-specific vector types} types. | ||
| \begin{note} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| %!TEX root = std.tex | ||
| %NOCHECKBEGIN(lib-*) | ||
| \rSec0[stmt]{Statements}% | ||
| \indextext{statement|(} | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| %!TEX root = std.tex | ||
| %NOCHECKBEGIN(lib-*) | ||
| \rSec0[temp]{Templates}% | ||
| \indextext{template|(} | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| %!TEX root = std.tex | ||
| %NOCHECKBEGIN(lib-ranges-*) | ||
|
|
||
| \rSec0[text]{Text processing library} | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| %!TEX root = std.tex | ||
| %NOCHECKBEGIN(lib-ranges-*) | ||
| \rSec0[time]{Time library} | ||
|
|
||
| \rSec1[time.general]{General} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| %!TEX root = std.tex | ||
| %NOCHECKBEGIN(lib-ranges-*) | ||
|
|
||
| \rSec0[utilities]{General utilities library} | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure
pyrightis a good call. There are other alternatives that do not require pulling in node as dependency.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just use it because it integrates nicely with VSCode out of the box. What would you recommend instead?