feat: added Python stub auto-generation with pyo3-introspection

[TimD1]

No description provided.

[RagnarGrootKoerkamp]

not sure if ready for review, so just some quick comments.

These types get automatically picked up by new pypi packages right? So no need to add extra instructions to the python section of the root-level readme?

(Although tbh I'm not sure how much my review will be worth anyway. If you test that it works that's probably sufficient.)

(also, feel free to contact me on discord; your collegues know my handle :) )

[TimD1]

clarification: the stub regeneration instructions I added to python/README.md, not the Python section of README.md. If there's another place this should live, just lmk.

Yes, the types should get picked up automatically by new PyPI packages. I'll test to make sure that works now. ✅

However, I also added a Python stub-regeneration check to CI to ensure they're kept up to date and working correctly. In order to do this check, the auto-generated .pyi file must be checked into git and compared against. This will need to be updated manually when the interface changes; hence the new instructions.

What I could do is modify the CI check to verify that the command succeeds but not check the output. Then we could remove the stub regeneration instructions. The only potential issue would be if the stub regeneration returns with a success code but doesn't generate the expected stubs, which may be unlikely enough that you'd prefer the simpler workflow. We'd need to modify the PyPI release script as well to regenerate the .pyi file if we're no longer storing it in git.

[RagnarGrootKoerkamp]

Ah right, if you can move the generation into the pypi release workflow, that does sound less intrusive for day-to-day development. (That is what you're suggesting right?) And then they don't have to be committed at all, but we could indeed test that generating them does succeed.

[RagnarGrootKoerkamp]

Since you're already working on the python side:

Any ideas on how to also ship bindings for the AVX-512 side of things?

This post has some options:

• commit to a single architecture
• runtime detection: this is hard because we use wide and it can only compile for 1 backend at a time in a single compiler invocation
• ship multiple versions of the crate

Do you think shipping eg sassy and sassy-512 sounds reasonable? At least it sounds like you would benefit from the 512-bit backend if you use the v2 searchers.

For the main binary release I'm going to use cargo-multivers which compiles the two versions of the binary into a single output, but I don't think this will work for libraries.

[TimD1]

I haven't worked with shipping bindings for multiple SIMD targets before, but I did just come across multiversion, which seems to do what you're looking for (including library support).

[RagnarGrootKoerkamp]

As far as I understand it, that crate will not work for us. I think it tells the compiler to auto-vectorize a given function separately for each target architecture, but we want to manually write specific SIMD-intrinsics. We also use wide, which selects a single SIMD-backend at compile time based on the target architecture features, and so unfortunately multiversion cannot do anything anymore at that point (at least not unless we do a massive rewrite).

[TimD1]

Ah right, if you can move the generation into the pypi release workflow, that does sound less intrusive for day-to-day development. (That is what you're suggesting right?) And then they don't have to be committed at all, but we could indeed test that generating them does succeed.

Yes! I moved the stub generation into the PyPI release workflow and kept the test.

Do you think shipping eg sassy and sassy-512 sounds reasonable? At least it sounds like you would benefit from the 512-bit backend if you use the v2 searchers.

I don't love any of the options above, but I think you're right. Shipping both sassy and sassy-512 appears to be the best option available.

[RagnarGrootKoerkamp]

Then the question is if we ship 1 package with sassy and sassy_512 libs, or 2 packages with sassy as lib. Probably the former, and then we can do import sassy512 as sassy or so

[RagnarGrootKoerkamp]

otherwise the python-stub code lgtm on a not-very-close look.

If you're happy to merge then so am I :)
We can do the AVX-512 lib separately

[znorgaard]

Could we add a mypy check afterwards? I think I'd vote for using the install uv action then run:

uv run \
    --with mypy \
    sh -c 'mypy python/sassy/example_typed.py'

This would confirm we didn't generate empty stubs but wouldn't be exhaustive.

[RagnarGrootKoerkamp]

sgtm.

But do we need uv for this? Ugh installing things in these CI containers is such a mess and I never have any idea if caching works or not for this. But maybe they already have mypy and just mypy path/... works?

[znorgaard]

That sounds reasonable too! Mostly, I know the uv approach works with a cache.

https://github.com/fulcrumgenomics/fgpyo/blob/ab9e9ec56670200319170a6b30327a8f973a99d5/.github/workflows/tests.yml#L28-L34

[RagnarGrootKoerkamp]

Then in should be fine :)

[TimD1]

I added in the cached uv, and set the Python version of the stub checks to 3.9 (3.8 doesn't support list type annotations used in the example_typed.py).

[RagnarGrootKoerkamp]

ready for merge then?

[TimD1]

Yes, this should be ready to merge.