-
Notifications
You must be signed in to change notification settings - Fork 274
Run stubtest to check stubs are complete
#1141
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: master
Are you sure you want to change the base?
Run stubtest to check stubs are complete
#1141
Conversation
stubs/test.sh
Outdated
|
|
||
| # Test the stubs for pyscipopt using stubtest | ||
| # This checks that the type hints in the stubs are consistent with the actual implementation | ||
| # Prerequisite: install mypy in same environment as pyscipopt and put stubtest in PATH |
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.
What would the error be when mypy is not installed? Also, can you be more explicit on what putting stubtest in PATH means?
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.
What would the error be when
mypyis not installed?
stubtest is distributed when you install mypy, so installing mypy is the only way to install stubtest.
Also, can you be more explicit on what putting stubtest in PATH means?
Yeah I agree that isn't clear, I just meant to activate the environment. Actually, I'll change the command to python -m stubtest, I think it's clearer
Are the |
No the |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1141 +/- ##
=======================================
Coverage 55.07% 55.07%
=======================================
Files 24 24
Lines 5420 5420
=======================================
Hits 2985 2985
Misses 2435 2435 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This PR adds a workflow that runs
stubteston the bundled type stubs. This means that whenever a class or method is added, the workflow will error until it is added to thescip.pyifile.Part of #1072
Type errors
A pre-requisite to running
stubtestis to havemypytype check the code without error. There were only a few errors reported bymypywhich I don't think are controversialAllow lists
For now, we rely on an allow list because the stubs are still wrong, they list all methods as taking
*args, **kwargsparameters which isn't true at runtime. The allow list silences all the known current errors until we manually fix them.There are 2 allow list files:
allowlistlists errors which are known and expected.todolists all the errors which we should aim to fix eventually.Entries
Each entry is a regex matching the fully qualified name of the object with an error. Each line can thus hide multiple errors. For instance, for the entry
pyscipopt.LP.addRow, these are the errors that get silenced:Conversely, some lines are actually duplicates. This is due to the re-exports. For instance, the entries:
point to the same underlying object, so both entries could be removed at the same time.
Maintenance
Finally, the workflow will also fail if an entry in the allow list does not generate any error, ensuring the allow list is always up to date.
How to fix?
The way forward is to fix all the errors in the
todoallow list. This means removing a couple of entries from the allow list, runningstubs/test.shand fixing any printed error.Unfortunately I don't know any way to automate this part.
stubtestdetects the argument names but it has no idea what annotation they should have, so some human judgment is required. Or a massive token budget for a LLM 😉