Skip to content

Conversation

makslevental
Copy link
Contributor

@makslevental makslevental commented Aug 28, 2025

This PR turns on automatic type stub generation (rather than relying on hand-written/updated stubs). It uses nanobind's stubgen facility.

Note, the only catch is we need a quite recent version of that code - specifically we need a commit of nanobind that includes wjakob/nanobind@53dce4e. In conventional requirements.txt files it's no issue to depend directly on a GitHub git commit (which I've done for mlir/python/requirements.txt) but such dependencies don't support pip hash (ironically). To workaround this I've temporarily updated .ci/monolithic-linux.sh and .ci/monolithic-windows.sh to explicitly install the appropriate commit of nanobind for our CI. When nanobind does its next PyPI release (which will contain the required 53dce4e commit), I'll update mlir/python/requirements.txt and .ci/all_requirements.txt and restore the monolithic scripts.

@makslevental makslevental force-pushed the users/makslevental/stubgen branch 30 times, most recently from 92b0650 to 7e6569c Compare August 30, 2025 01:53
@makslevental
Copy link
Contributor Author

@ingomueller-net
Copy link
Contributor

This is awesome!

Do we know if the generated stubs are correct? I have set up a CI job for a small MLIR project that uses pyright to check the types and it took me quite some effort to make that pass. From memories, I had to define some signatures by hand because stubgen didn't generate the full module prefix of the types and a file with fix-up patterns because either __str__ or __repr__ of some enum class wasn't generated with the correct signature. I now think that the manual signatures might not be necessary and could instead be made correct with a few generic patterns but I haven't tried yet. TL;DR: I think we should make sure that the annotations from stubgen are correct; otherwise, their usefulness might be quite limited and actually be worse than the manually written ones.

Also, maybe we can ask the nanobind devs if they have an estimate on when the new release comes out and wait for that to happen.

@makslevental
Copy link
Contributor Author

had to define some signatures by hand because stubgen didn't generate the full module prefix of the types and a file with fix-up patterns because either str or repr of some enum class wasn't generated with the correct signature. I now think that the manual signatures substrait-io/substrait-mlir-contrib#171 and could instead be made correct with a few generic patterns

Yea I saw you'd hand-written some signatures and I was planning on doing the same (though in a follow-up PR). But the patterns file looks like a winner so I'll do that instead.

Also, maybe we can ask the nanobind devs if they have an estimate on when the new release comes out and wait for that to happen.

Yea I asked and it's like 1-2 weeks away so we could wait - I'm just impatient sometimes :)

@makslevental makslevental force-pushed the users/makslevental/stubgen branch from 655e9bf to 8395098 Compare September 4, 2025 16:52
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