Skip to content
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

[Bug]: Type of Axes is unknown pyright #28022

Closed
haarisr opened this issue Apr 4, 2024 · 5 comments · Fixed by #28230
Closed

[Bug]: Type of Axes is unknown pyright #28022

haarisr opened this issue Apr 4, 2024 · 5 comments · Fixed by #28230
Milestone

Comments

@haarisr
Copy link
Contributor

haarisr commented Apr 4, 2024

Bug summary

plt.axes should return Axes and not matplotlib.axes.Axes

Code for reproduction

@_docstring.dedent_interpd
def axes(
    arg: None | tuple[float, float, float, float] = None,
    **kwargs
) -> matplotlib.axes.Axes: #axes is not a known member of module 'matplotlib' #type of axes is unknown #type of Axes is unknown

Actual outcome

Same as above

Expected outcome

@_docstring.dedent_interpd
def axes(
    arg: None | tuple[float, float, float, float] = None,
    **kwargs
) -> Axes:

Additional information

No response

Operating system

No response

Matplotlib Version

3.8.3

Matplotlib Backend

No response

Python version

3.12

Jupyter version

No response

Installation

pip

@timhoffm
Copy link
Member

timhoffm commented Apr 4, 2024

The technical location of the class is matplotlib.axes._axes.Axes but it's re-imported in matplotlib.axes, which is our canonical public point of contact. Maybe pyright is stumbling over that?

@haarisr
Copy link
Contributor Author

haarisr commented Apr 4, 2024

Pyright understands matplotlib and goes under __init__.py but cannot find any source of axes there.

Why not use Axes as the return type instead? It is imported in pyplot.py too. The same is used for functions returning Figure. I can make a PR if this is the intended direction.

@timhoffm
Copy link
Member

timhoffm commented Apr 4, 2024

Yes, changing matplotlib.axes.Axes to Axes in type annotations in pyplot seems the right thing to do. ping @ksunden do you agree? A PR is welcome!

@ksunden
Copy link
Member

ksunden commented Apr 4, 2024

See full explanations here:

Short version is that sphinx had trouble resolving some things, so I resorted to the fully specified names in some cases. I think this particular example should be fine to be shortened, but the other places in pyplot.py where Axes was referred to as a union type broke sphinx in ways that I could not at the time figure out how to resolve.

It is possible sphinx has changed how it resolves links in type hints since then, I have not checked.

That said, this reference should work for type checkers, I'm pretty sure (and does work for mypy for instance, which is what we use for our CI).

It is true that matplotlib.axes is not imported by default:

>>> import matplotlib
>>> matplotlib.axes
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/kyle/src/scipy/matplotlib/lib/matplotlib/_api/__init__.py", line 217, in __getattr__
    raise AttributeError(
AttributeError: module 'matplotlib' has no attribute 'axes'

Adding the explicit imports (I just did it in the if TYPE_CHECKING: block since its working around type checker limitations) appears to fix the problems (for some reason did not fix one of the backend_bases flags, not sure why):

diff --git a/lib/matplotlib/pyplot.py b/lib/matplotlib/pyplot.py
index 2376c62439..426892afca 100644
--- a/lib/matplotlib/pyplot.py
+++ b/lib/matplotlib/pyplot.py
@@ -90,6 +90,9 @@ if TYPE_CHECKING:
     import PIL.Image
     from numpy.typing import ArrayLike
 
+    import matplotlib.axes
+    import matplotlib.artist
+    import matplotlib.backend_bases
     from matplotlib.axis import Tick
     from matplotlib.axes._base import _AxesBase
     from matplotlib.backend_bases import RendererBase, Event

The remaining flags I see from pyright in pyplot.py are largely due to either:

a) types are narrowed in ways pyright doesn't (or can't) know e.g.:

  • we looked up an rcParam, so we know the type narrowed
  • we do a hasattr check before accessing an optional attribute

b) a small handful of artist functions that do not get type hints because of the complexity of doing so (its not impossible, just never got around to it and didn't hold the rest of the work on it)

I have no issue with paring down the number of flags we get with pyright, though mypy remains my preference for CI purposes for now (not totally opposed to both, but did not optimize for pyright). There were ~3000 flags from pyright that mypy did not flag when we first added type hints (10k if you include tests), so not trivial.

I do use a pyright-based LSP/editor integration, but it doesn't play well with editable meson installs, so mostly don't get mpl flags unless I go looking for them.

@haarisr
Copy link
Contributor Author

haarisr commented Apr 5, 2024

I like the if TYPE_CHECKING solution better. It works for now and would not bother sphinx as well.

Let me know if you want a PR for that.

Off topic: I prefer pyright for now as it does not hang up once an error is encountered unlike pyright. Also it seems to have support for PEP 695 generics, and it has some strong arguments in its comparison to mypy. I would prefer mypy but it automatically skips if a type cannot be inferred and that to me is a big bummer. Pyright on the other hand does not need to append None on functions that do not return anything which is a bit less clutter imo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants