Skip to content

Ensure tinypages ignored by mypy/stubtest #25587

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

Merged
merged 1 commit into from
Apr 4, 2023

Conversation

ksunden
Copy link
Member

@ksunden ksunden commented Mar 31, 2023

Should mean that the figure no longer gets shown when running stubtest. I had previously been working around by setting the MPLBACKEND envvar to a non-gui backend

PR Summary

PR Checklist

Documentation and Tests

  • Has pytest style unit tests (and pytest passes)
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • New plotting related features are documented with examples.

Release Notes

  • New features are marked with a .. versionadded:: directive in the docstring and documented in doc/users/next_whats_new/
  • API changes are marked with a .. versionchanged:: directive in the docstring and documented in doc/api/next_api_changes/
  • Release notes conform with instructions in next_whats_new/README.rst or next_api_changes/README.rst

Should mean that the figure no longer gets shown when running stubtest. I had previously been working around by setting the MPLBACKEND envvar to a non-gui backend
@QuLogic
Copy link
Member

QuLogic commented Mar 31, 2023

I guess there isn't a way to ignore this for only stubtest and not mypy (which I assume wouldn't import since it's static)?

@tacaswell tacaswell added this to the v3.8.0 milestone Mar 31, 2023
@ksunden
Copy link
Member Author

ksunden commented Mar 31, 2023

Yeah, stubtest has config to filter out identified warnings, but not to avoid.

Technically possible to have a separate config file, but having parallel mostly the same configs seems like a poor choice. Especially when mypy the thing being ignored has no type hints, so mypy is rather pointless on that file (and even if we add type hints to tinypages at some point, it is so short, disconnected from other code, and rarely changed that a manual check is not so bad)

@jklymak
Copy link
Member

jklymak commented Mar 31, 2023

it is so short, disconnected from other code, and rarely changed that a manual check is not so bad

But #25515! Though I don't think that type checking is at all relevant in here ;-)

@ksunden
Copy link
Member Author

ksunden commented Mar 31, 2023

I mean yeah (I know you were mostly joking, but that PR actually reinforces my point if anything). The only changed file from #25515 affected by the ignore introduced here is the conf.py (which still doesn't have type hints, so mypy checking it is rather unnecessary). The rst file changes would always be ignored by mypy/stubtest/etc.

I could have done a bit more targeted of an ignore (only the tinypages example with plt.show() was actually problematic, specifically because stubtest imports everything), but the whole folder is untyped, so may as well, and cross the bridge of enabling checks if/when it becomes typed. Being a bit more broad means the changes to tinypages (like #25515) can happen more freely until then.

@QuLogic QuLogic merged commit 87a2b9d into matplotlib:main Apr 4, 2023
@ksunden ksunden deleted the stubtest_fig_open branch April 4, 2023 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants