From a711b12b0866a721686be49b7a694bae3abd29cc Mon Sep 17 00:00:00 2001 From: Kyle Sunden Date: Tue, 26 Sep 2023 15:26:11 -0500 Subject: [PATCH 1/4] Add tool for running stubtest Walks the ast to find delete_parameter calls and automatically ignores them as their signature is modified at runtime Could potentially be further modified to respect the alias behavior, which is the majority of the current allowlist Makes running stubtest easier as now it is just 'python tools/stubtest.py', rather than a long incantation with multiple arguments --- .github/workflows/mypy-stubtest.yml | 5 +-- ci/mypy-stubtest-allowlist.txt | 3 -- doc/devel/contribute.rst | 16 ++++----- tools/stubtest.py | 53 +++++++++++++++++++++++++++++ 4 files changed, 60 insertions(+), 17 deletions(-) create mode 100644 tools/stubtest.py diff --git a/.github/workflows/mypy-stubtest.yml b/.github/workflows/mypy-stubtest.yml index 6da6f607642c..4cb225258f4f 100644 --- a/.github/workflows/mypy-stubtest.yml +++ b/.github/workflows/mypy-stubtest.yml @@ -37,10 +37,7 @@ jobs: REVIEWDOG_GITHUB_API_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | set -o pipefail - MPLBACKEND=agg python -m mypy.stubtest \ - --mypy-config-file pyproject.toml \ - --allowlist ci/mypy-stubtest-allowlist.txt \ - matplotlib | \ + MPLBACKEND=agg python tools/stubtest.py | \ reviewdog \ -efm '%Eerror: %m' \ -efm '%CStub: in file %f:%l' \ diff --git a/ci/mypy-stubtest-allowlist.txt b/ci/mypy-stubtest-allowlist.txt index e0890b3f7117..5474d6fc5087 100644 --- a/ci/mypy-stubtest-allowlist.txt +++ b/ci/mypy-stubtest-allowlist.txt @@ -55,13 +55,10 @@ matplotlib.cm.unregister_cmap matplotlib.collections.PolyCollection.span_where # 3.8 deprecations -matplotlib.cbook.get_sample_data matplotlib.contour.ContourSet.allkinds matplotlib.contour.ContourSet.allsegs matplotlib.contour.ContourSet.tcolors matplotlib.contour.ContourSet.tlinewidths -matplotlib.ticker.LogLocator.__init__ -matplotlib.ticker.LogLocator.set_params # positional-only argument name lacking leading underscores matplotlib.axes._base._AxesBase.axis diff --git a/doc/devel/contribute.rst b/doc/devel/contribute.rst index 9b53a80ab374..85c061b7ce8f 100644 --- a/doc/devel/contribute.rst +++ b/doc/devel/contribute.rst @@ -425,11 +425,7 @@ Introducing updated on introduction. - Items decorated with ``@_api.delete_parameter`` should include a default value hint for the deleted parameter, even if it did not previously have one (e.g. - ``param: = ...``). Even so, the decorator changes the default value to a - sentinel value which should not be included in the type stub. Thus, Mypy Stubtest - needs to be informed of the inconsistency by placing the method into - :file:`ci/mypy-stubtest-allowlist.txt` under a heading indicating the deprecation - version number. + ``param: = ...``). Expiring ~~~~~~~~ @@ -452,11 +448,11 @@ Expiring will have been updated at introduction, and require no change now. - Items decorated with ``@_api.delete_parameter`` will need to be updated to the final signature, in the same way as the ``.py`` file signature is updated. - The entry in :file:`ci/mypy-stubtest-allowlist.txt` should be removed. - - Any other entries in :file:`ci/mypy-stubtest-allowlist.txt` under a version's - deprecations should be double checked, as only ``delete_parameter`` should normally - require that mechanism for deprecation. For removed items that were not in the stub - file, only deleting from the allowlist is required. + - Any entries in :file:`ci/mypy-stubtest-allowlist.txt` which indicate a deprecation + version should be double checked. In most cases this is not needed, though some + items were never type hinted in the first place and were added to this file + instead. For removed items that were not in the stub file, only deleting from the + allowlist is required. Adding new API -------------- diff --git a/tools/stubtest.py b/tools/stubtest.py new file mode 100644 index 000000000000..a68f3ab5c3b3 --- /dev/null +++ b/tools/stubtest.py @@ -0,0 +1,53 @@ +import ast +import pathlib +import subprocess +import sys +import tempfile + +root = pathlib.Path(__file__).parent.parent + +lib = root / "lib" +mpl = lib / "matplotlib" + + +class Visitor(ast.NodeVisitor): + def __init__(self, filepath, output): + self.filepath = filepath + self.context = list(filepath.with_suffix("").relative_to(lib).parts) + self.output = output + + def visit_FunctionDef(self, node): + if any("delete_parameter" in ast.unparse(line) for line in node.decorator_list): + parents = [] + if hasattr(node, "parent"): + parent = node.parent + while hasattr(parent, "parent") and not isinstance(parent, ast.Module): + parents.append(parent.name) + parent = parent.parent + self.output.write(f"{'.'.join(self.context + parents)}.{node.name}\n") + + +with tempfile.NamedTemporaryFile("wt") as f: + for path in mpl.glob("**/*.py"): + v = Visitor(path, f) + tree = ast.parse(path.read_text()) + + # Assign parents to tree so they can be backtraced + for node in ast.walk(tree): + for child in ast.iter_child_nodes(node): + child.parent = node + + v.visit(tree) + f.flush() + proc = subprocess.run( + [ + "stubtest", + "--mypy-config-file=pyproject.toml", + "--allowlist=ci/mypy-stubtest-allowlist.txt", + f"--allowlist={f.name}", + "matplotlib", + ], + cwd=root, + ) + +sys.exit(proc.returncode) From a59c2680dc176744282be1173f6a90cfb475d909 Mon Sep 17 00:00:00 2001 From: Kyle Sunden Date: Tue, 26 Sep 2023 17:47:27 -0500 Subject: [PATCH 2/4] Add alias decorator handling to the generated allowlist --- ci/mypy-stubtest-allowlist.txt | 82 ---------------------------------- tools/stubtest.py | 28 +++++++++++- 2 files changed, 27 insertions(+), 83 deletions(-) diff --git a/ci/mypy-stubtest-allowlist.txt b/ci/mypy-stubtest-allowlist.txt index 5474d6fc5087..4cbbcac94dcf 100644 --- a/ci/mypy-stubtest-allowlist.txt +++ b/ci/mypy-stubtest-allowlist.txt @@ -63,88 +63,6 @@ matplotlib.contour.ContourSet.tlinewidths # positional-only argument name lacking leading underscores matplotlib.axes._base._AxesBase.axis -# Aliases (dynamically generated, not type hinted) -matplotlib.collections.Collection.get_aa -matplotlib.collections.Collection.get_antialiaseds -matplotlib.collections.Collection.get_dashes -matplotlib.collections.Collection.get_ec -matplotlib.collections.Collection.get_edgecolors -matplotlib.collections.Collection.get_facecolors -matplotlib.collections.Collection.get_fc -matplotlib.collections.Collection.get_linestyles -matplotlib.collections.Collection.get_linewidths -matplotlib.collections.Collection.get_ls -matplotlib.collections.Collection.get_lw -matplotlib.collections.Collection.get_transOffset -matplotlib.collections.Collection.set_aa -matplotlib.collections.Collection.set_antialiaseds -matplotlib.collections.Collection.set_dashes -matplotlib.collections.Collection.set_ec -matplotlib.collections.Collection.set_edgecolors -matplotlib.collections.Collection.set_facecolors -matplotlib.collections.Collection.set_fc -matplotlib.collections.Collection.set_linestyles -matplotlib.collections.Collection.set_linewidths -matplotlib.collections.Collection.set_ls -matplotlib.collections.Collection.set_lw -matplotlib.collections.Collection.set_transOffset -matplotlib.lines.Line2D.get_aa -matplotlib.lines.Line2D.get_c -matplotlib.lines.Line2D.get_ds -matplotlib.lines.Line2D.get_ls -matplotlib.lines.Line2D.get_lw -matplotlib.lines.Line2D.get_mec -matplotlib.lines.Line2D.get_mew -matplotlib.lines.Line2D.get_mfc -matplotlib.lines.Line2D.get_mfcalt -matplotlib.lines.Line2D.get_ms -matplotlib.lines.Line2D.set_aa -matplotlib.lines.Line2D.set_c -matplotlib.lines.Line2D.set_ds -matplotlib.lines.Line2D.set_ls -matplotlib.lines.Line2D.set_lw -matplotlib.lines.Line2D.set_mec -matplotlib.lines.Line2D.set_mew -matplotlib.lines.Line2D.set_mfc -matplotlib.lines.Line2D.set_mfcalt -matplotlib.lines.Line2D.set_ms -matplotlib.patches.Patch.get_aa -matplotlib.patches.Patch.get_ec -matplotlib.patches.Patch.get_fc -matplotlib.patches.Patch.get_ls -matplotlib.patches.Patch.get_lw -matplotlib.patches.Patch.set_aa -matplotlib.patches.Patch.set_ec -matplotlib.patches.Patch.set_fc -matplotlib.patches.Patch.set_ls -matplotlib.patches.Patch.set_lw -matplotlib.text.Text.get_c -matplotlib.text.Text.get_family -matplotlib.text.Text.get_font -matplotlib.text.Text.get_font_properties -matplotlib.text.Text.get_ha -matplotlib.text.Text.get_name -matplotlib.text.Text.get_size -matplotlib.text.Text.get_style -matplotlib.text.Text.get_va -matplotlib.text.Text.get_variant -matplotlib.text.Text.get_weight -matplotlib.text.Text.set_c -matplotlib.text.Text.set_family -matplotlib.text.Text.set_font -matplotlib.text.Text.set_font_properties -matplotlib.text.Text.set_ha -matplotlib.text.Text.set_ma -matplotlib.text.Text.set_name -matplotlib.text.Text.set_size -matplotlib.text.Text.set_stretch -matplotlib.text.Text.set_style -matplotlib.text.Text.set_va -matplotlib.text.Text.set_variant -matplotlib.text.Text.set_weight -matplotlib.axes._base._AxesBase.get_fc -matplotlib.axes._base._AxesBase.set_fc - # Other dynamic python behaviors not type hinted matplotlib.rcsetup.defaultParams diff --git a/tools/stubtest.py b/tools/stubtest.py index a68f3ab5c3b3..3f3c6931255f 100644 --- a/tools/stubtest.py +++ b/tools/stubtest.py @@ -1,4 +1,5 @@ import ast +import os import pathlib import subprocess import sys @@ -22,10 +23,34 @@ def visit_FunctionDef(self, node): if hasattr(node, "parent"): parent = node.parent while hasattr(parent, "parent") and not isinstance(parent, ast.Module): - parents.append(parent.name) + parents.insert(0, parent.name) parent = parent.parent self.output.write(f"{'.'.join(self.context + parents)}.{node.name}\n") + def visit_ClassDef(self, node): + for dec in node.decorator_list: + if "define_aliases" in ast.unparse(dec): + parents = [] + if hasattr(node, "parent"): + parent = node.parent + while hasattr(parent, "parent") and not isinstance( + parent, ast.Module + ): + parents.insert(0, parent.name) + parent = parent.parent + aliases = ast.literal_eval(dec.args[0]) + # Written as a regex rather than two lines to avoid unused entries + # for setters on items with only a getter + for substitutions in aliases.values(): + parts = self.context + parents + [node.name] + self.output.write( + "\n".join( + f"{'.'.join(parts)}.[gs]et_{a}\n" for a in substitutions + ) + ) + for child in ast.iter_child_nodes(node): + self.visit(child) + with tempfile.NamedTemporaryFile("wt") as f: for path in mpl.glob("**/*.py"): @@ -48,6 +73,7 @@ def visit_FunctionDef(self, node): "matplotlib", ], cwd=root, + env=os.environ | {"MPLBACKEND": "agg"}, ) sys.exit(proc.returncode) From 61661f1c40fcaafe19b1f230da83d569a05f4ff9 Mon Sep 17 00:00:00 2001 From: Kyle Sunden Date: Wed, 27 Sep 2023 17:18:49 -0500 Subject: [PATCH 3/4] Close file so it hopefully works on windows (but not tested) --- tools/stubtest.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tools/stubtest.py b/tools/stubtest.py index 3f3c6931255f..71de6bda20c8 100644 --- a/tools/stubtest.py +++ b/tools/stubtest.py @@ -52,7 +52,7 @@ def visit_ClassDef(self, node): self.visit(child) -with tempfile.NamedTemporaryFile("wt") as f: +with tempfile.NamedTemporaryFile("wt", delete=False) as f: for path in mpl.glob("**/*.py"): v = Visitor(path, f) tree = ast.parse(path.read_text()) @@ -64,6 +64,7 @@ def visit_ClassDef(self, node): v.visit(tree) f.flush() + f.close() proc = subprocess.run( [ "stubtest", @@ -75,5 +76,9 @@ def visit_ClassDef(self, node): cwd=root, env=os.environ | {"MPLBACKEND": "agg"}, ) + try: + os.unlink(f.name) + except OSError: + pass sys.exit(proc.returncode) From 06e47316ed527d10be9199774e43d55bcedc527d Mon Sep 17 00:00:00 2001 From: Kyle Sunden Date: Mon, 2 Oct 2023 11:37:19 -0500 Subject: [PATCH 4/4] Switch to tempdirectory for windows support and autocleanup --- tools/stubtest.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tools/stubtest.py b/tools/stubtest.py index 71de6bda20c8..7c93d2dae157 100644 --- a/tools/stubtest.py +++ b/tools/stubtest.py @@ -52,25 +52,25 @@ def visit_ClassDef(self, node): self.visit(child) -with tempfile.NamedTemporaryFile("wt", delete=False) as f: - for path in mpl.glob("**/*.py"): - v = Visitor(path, f) - tree = ast.parse(path.read_text()) +with tempfile.TemporaryDirectory() as d: + p = pathlib.Path(d) / "allowlist.txt" + with p.open("wt") as f: + for path in mpl.glob("**/*.py"): + v = Visitor(path, f) + tree = ast.parse(path.read_text()) - # Assign parents to tree so they can be backtraced - for node in ast.walk(tree): - for child in ast.iter_child_nodes(node): - child.parent = node + # Assign parents to tree so they can be backtraced + for node in ast.walk(tree): + for child in ast.iter_child_nodes(node): + child.parent = node - v.visit(tree) - f.flush() - f.close() + v.visit(tree) proc = subprocess.run( [ "stubtest", "--mypy-config-file=pyproject.toml", "--allowlist=ci/mypy-stubtest-allowlist.txt", - f"--allowlist={f.name}", + f"--allowlist={p}", "matplotlib", ], cwd=root,