Skip to content

Annotate several Model methods in the stubs#1220

Open
jonathanberthias wants to merge 2 commits into
scipopt:masterfrom
jonathanberthias:model-annotations-1
Open

Annotate several Model methods in the stubs#1220
jonathanberthias wants to merge 2 commits into
scipopt:masterfrom
jonathanberthias:model-annotations-1

Conversation

@jonathanberthias
Copy link
Copy Markdown
Contributor

This is a first batch of annotations and default parameter values. I believe these are rather straightforward.

I ignored some annotations on purpose: anything related to ExprCons because the annotations for Expr operations aren't correct yet, and anything related to the PY_SCIP_* enums. These can be revisited at a later stage.

Part of #1072

@Joao-Dionisio
Copy link
Copy Markdown
Member

Awesome @jonathanberthias ! Can you also adapt the stub checker for this, to force new contributions to annotate as well?

@jonathanberthias
Copy link
Copy Markdown
Contributor Author

Awesome @jonathanberthias ! Can you also adapt the stub checker for this, to force new contributions to annotate as well?

I'm not sure that's possible yet, I only annotated a tiny part of the API so we would need to "finish" everything before we can forbid broad annotations like ˋIncompleteˋ.

BTW the automated script to generate stubs can't really be used anymore since it would remove all of these new annotations. We either need to make it smarter and not remove existing annotations, or rely on manual work for now.

@Joao-Dionisio
Copy link
Copy Markdown
Member

so we would need to "finish" everything before we can forbid broad annotations like ˋIncompleteˋ.

That is true, but perhaps we could have an intermediate script that just checks that no new stub errors are introduced. But maybe it's not worth the effort, yes.

On the automated script: yeah, I should do another pass on it someday. Not this weekend, but perhaps the next.

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.

2 participants