sizmailov / pybind11-stubgen

Generate stubs for python modules
Other
228 stars 45 forks source link

buffer type broken by #168 #173

Closed TheTripleV closed 10 months ago

TheTripleV commented 10 months ago

buffer now seems to look for typing.Buffer (which doesn't exist) and not typing_extensions.Buffer.

sizmailov commented 10 months ago

Maybe you use a dated version. Tests cover this.

https://github.com/sizmailov/pybind11-stubgen/blob/2f969a11635dfd7d9abacabc2832cecf9f9b1359/tests/py-demo/bindings/src/modules/typing.cpp#L5

https://github.com/sizmailov/pybind11-stubgen/blob/2f969a11635dfd7d9abacabc2832cecf9f9b1359/tests/stubs/python-3.7/pybind11-master/demo/_bindings/typing.pyi#L9

https://github.com/sizmailov/pybind11-stubgen/blob/2f969a11635dfd7d9abacabc2832cecf9f9b1359/tests/stubs/python-3.12/pybind11-master/demo/_bindings/typing.pyi#L9

sizmailov commented 10 months ago

Ok, I forgot about case-sensitivity. :disappointed:

sizmailov commented 10 months ago

CI tests against pybind11's master branch, which has capitalized Buffer in docstrings. I should probably include older pybind11 version to CI :(

virtuald commented 10 months ago

That's weird, because I updated our pybind11 recently (but we use smart_holder)... which has b4573674bc65a7842b49be45d63b582f4c15d1b4 in it?

commit 7953d19a7c17ff1bfe0b7c4cdfde216d400d5f28 (HEAD -> 2024, origin/smart_holder)
Merge: 9110b766 2b2e4ca4
Author: Ralf W. Grosse-Kunstleve <rwgk@google.com>
Date:   Tue Oct 3 22:45:09 2023 -0700

    Merge branch 'master' into sh_merge_master

...

commit b4573674bc65a7842b49be45d63b582f4c15d1b4
Author: Sergei Izmailov <sergei.a.izmailov@gmail.com>
Date:   Wed Sep 13 04:47:39 2023 +0900

    Update render for buffer sequence and handle  (#4831)

I see 2.3.4 failing at https://github.com/robotpy/robotpy-wpiutil/actions/runs/6609220896/job/17949256791#step:8:547

virtuald commented 10 months ago

Ah, our span_caster outputs a lowercase buffer (https://github.com/robotpy/robotpy-wpiutil/blob/2dd9b691b7dc776fd266f04f4b83bef45dfdef23/wpiutil/src/type_casters/wpi_span_type_caster.h#L148)... but I suppose you should support either the older or newer version.

This is actually the third type caster related issue that I've ran into where the name was deceiving me. I don't think there's a way for stubgen to know if the type has a custom caster associated with it -- if it did, that would be really useful to output in the error message.

sizmailov commented 10 months ago

I don't think there's a way for stubgen

That's correct, stubgen is unaware of casters

ax3l commented 10 months ago

Thank you all for the issue and quick response :+1:

CI tests against pybind11's master branch, which has capitalized Buffer in docstrings. I should probably include older pybind11 version to CI :(

@sizmailov Do you think it is easy to test in CI / support in stubgen the latest stable pybind11 release (besides master)? :)

sizmailov commented 10 months ago

@ax3l I can add v2.11 branch to CI if it's what you are asking for.

ax3l commented 10 months ago

Thank you, that sounds great and was what I was thinking.

sizmailov commented 10 months ago

I forgot to make GitHub release, which creates a tag which triggers upload to PyPI. So v2.3.5 was on gihub, but not in PyPI