Skip to content

Conversation

@jl-wynen
Copy link
Member

As discussed on Slack, the stubgen is of limited use now:

I am running into more and more subtleties with generating the stub file. The latest troublemakers are overloaded functions that are bound dynamically (e.g., hist or get). The stubgen cannot see the overloads without parsing the full modules where those functions are defined. It only sees the function that has been bound to, e.g., Variable. And that does not have the required type information. I implemented a whole heap of mechanisms to work around limitations like this. But it is getting pretty complex.

So here is my new proposal: Get rid of the stub generator.
It has served its purpose for building most of the stubs. But now we can edit the file manually and encode whatever complex type dependencies we have. We will likely not change the class interfaces much in the future. So maintaining the stub by hand should not be much work.
As a counter argument, we risk the stubs getting out of sync with the actual classes. But this should be caught by running mypy on our code base, including the tests. (Assuming that we use and test every function and all their arguments that we implement.) Plus, the stubs are already out of sync.

This PR

  • removes the stubgen.
  • adds a runtime test that checks that the stubs are consistent with the runtime classes.
  • reformats the stub file using ruff as people will now edit the file manually.
  • Fixes small issues in the stub to make the new test pass. Other issues are left to be fixed later, e.g., in Fix typing issues #3495.

When reviewing cpp_classes.pyi, ignore the reformatting commit e4b4d08 to see actual changes to the stubs.

@jl-wynen jl-wynen assigned jl-wynen and unassigned jl-wynen Jul 10, 2024
@jl-wynen jl-wynen mentioned this pull request Jul 11, 2024
@jokasimr jokasimr self-assigned this Jul 12, 2024
Copy link
Contributor

@jokasimr jokasimr left a comment

Choose a reason for hiding this comment

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

Nice. If I update the interface of a core class without editing the stub, would the tests or mypy catch the inconsistency?

@jl-wynen
Copy link
Member Author

jl-wynen commented Jul 12, 2024

The new test should catch it if you add or remove a function. It does not catch changes in function signatures. And mypy should catch any mistakes when you use the new changed interface anywhere (that is being checked by mypy).

@jl-wynen jl-wynen merged commit dddd40a into main Jul 12, 2024
@jl-wynen jl-wynen deleted the remove-stubgen branch July 12, 2024 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants