-
Notifications
You must be signed in to change notification settings - Fork 271
Initialize stubs #1073
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?
Initialize stubs #1073
Conversation
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.
Pull Request Overview
This PR introduces type stubs for the PySCIPOpt package to improve type checking and IDE support. The stubs were generated using stubgen
from the Mypy project and then manually refined to fix issues and add type annotations to function parameters.
- Type stubs provide LSP/type checker awareness of PySCIPOpt package structure
- Parameter type annotations added to recipe functions for better type safety
- Import statements updated to use explicit re-exports with type annotations
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
src/pyscipopt/scip.pyi | Main type stub file containing all class and function signatures with placeholder types |
src/pyscipopt/recipes/nonlinear.py | Added type annotation to sense parameter |
src/pyscipopt/recipes/infeasibilities.py | Added type annotation to verbose parameter |
src/pyscipopt/_version.py | Added type annotation to __version__ variable |
src/pyscipopt/init.py | Updated imports to use explicit re-export syntax for better type checking |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
n_runs: int = None | ||
n_nodes: int = None |
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.
These dataclass fields should use Optional[int]
and Optional[float]
instead of = None
with non-optional types. The current syntax creates type inconsistency where the type hint says int
/float
but the default value is None
.
Copilot uses AI. Check for mistakes.
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.
Absolutely, but there a lot of things to fix beyond this trivial change 😀
first_solution: float = None | ||
primal_bound: float = None | ||
dual_bound: float = None | ||
gap: float = None | ||
primal_dual_integral: float = None |
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.
These dataclass fields should use Optional[int]
and Optional[float]
instead of = None
with non-optional types. The current syntax creates type inconsistency where the type hint says int
/float
but the default value is None
.
Copilot uses AI. Check for mistakes.
Hi @Joao-Dionisio! This is the first PR to set some things up. It's 99% autogenerated stubs using
stubgen
(part of the Mypy project), and then I just fixed a couple easy things.This should be enough for LSPs/type checkers to know what exists in the PySCIPOpt package, but the types themselves are currently useless since nearly all methods take
*args, **kwargs
.I separated everything I did into small commits so you can see what I did exactly. The first commit is the raw output from
stubgen
, the others fix some things here and there but nothing important.One note on the organization: the stubs should mirror the structure of the final package. In this case, it means almost everything is in
scip.pyi
.I didn't include docstrings. I do have a script in my repo that parses the source Cython files with regexes and adds the docstrings to the stubs, but I recently found out about astral-sh/docstring-adder and I would like to see if that can be applied to this project. If so, it is a much more trustworthy approach since it relies on runtime objects and not on fragile manual parsing.
In any case, I'll tackle that in another PR 😄