FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Post stubtest results to GitHub checks by QuLogic · Pull Request #25586 · matplotlib/matplotlib · GitHub

Repository navigation

Post stubtest results to GitHub checks - #25586

Merged
ksunden merged 1 commit into
matplotlib:mainfrom
QuLogic:reviewdog-mypy-stubtest
Apr 17, 2023
Merged

ksunden merged 1 commit into
matplotlib:mainfrom
QuLogic:reviewdog-mypy-stubtest

Conversation

QuLogic commented Mar 31, 2023

Copy link
Copy Markdown
Member

PR Summary

Unfortunately, the way that stubtest outputs its error messages makes it difficult to post a message on both the .py and .pyi stub, so we decided to take the full message and post it only on one.

As a test, I have made PR in my repo QuLogic#30 which started from the previously-unfixed #25542, and then I also added a commit with multiple argument mismatches to confirm that stubtest didn't output some different format.

Results are at the bottom of https://github.com/QuLogic/matplotlib/pull/30/files

PR Checklist

Documentation and Tests

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

Release Notes

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

QuLogic added topic: typing CI: testing CI configuration and testing labels Mar 31, 2023
QuLogic added this to the v3.8.0 milestone Mar 31, 2023

ksunden commented Mar 31, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

The case that may need to be looked at a bit is the missing stub case, as that does not have a stub path to parse.

Edit: on second thought, it may give the path of the parent object or just line 0 of the module, but still worth a look

QuLogic commented Mar 31, 2023

Copy link
Copy Markdown
Member Author

Would that be for a new Python file entirely?

ksunden commented Mar 31, 2023

Copy link
Copy Markdown
Member

No, just for a new method is what I was thinking

QuLogic commented Apr 1, 2023

Copy link
Copy Markdown
Member Author

The stubtest output for that case doesn't include a line number, so it doesn't get posted, but the other errors do work fine. I'm not sure if I can get it to fall back to posting on the .py or if it would confuse it.

QuLogic commented Apr 1, 2023

Copy link
Copy Markdown
Member Author

I also just added an extra function in the stub, but not the Python file, and I see that that doesn't get posted either; it looks like there's a slightly difference in output in that case, which I should be able to cover.

QuLogic force-pushed the reviewdog-mypy-stubtest branch from eff69d5 to 2d05200 Compare April 6, 2023 08:06

QuLogic commented Apr 6, 2023

Copy link
Copy Markdown
Member Author

OK, this is now able to post 1) differences between stubs and runtime; 2) items in stubs but not in runtime. Unfortunately, the case with items in runtime but not stubs cannot be caught, AFAICT.

See https://github.com/QuLogic/matplotlib/pull/30/files

QuLogic commented Apr 6, 2023

Copy link
Copy Markdown
Member Author

I opened an issue on stubtest to see if concise error messages could contain a bit more information (linked above). This could simplify this posting process as message would not be multiline.

oscargus left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Let's try this and see what happens.

ksunden merged commit eb3fa7e into matplotlib:main Apr 17, 2023
QuLogic deleted the reviewdog-mypy-stubtest branch April 17, 2023 22:44
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI: testing CI configuration and testing topic: typing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants


Back | FazBrowse Home | New Git URL