apache / arrow-nanoarrow

Helpers for Arrow C Data & Arrow C Stream interfaces
https://arrow.apache.org/nanoarrow
Apache License 2.0
174 stars 38 forks source link

chore(python): Fix type stubs for updated module structure #606

Closed paleolimbot closed 2 months ago

paleolimbot commented 2 months ago

This PR updates the type stubs for the Cython modules to reflect the updated structure. I don't think this mechanism for generating stubs is the best long-term strategy (requires remembering to do so!)...we could also do something like omit the generated stubs from the source tree and generate them only before packaging; however, this would make them not useful for local development (which is one of the primary short-term benefits of including them).

paleolimbot commented 2 months ago

@jorisvandenbossche I know you've looked at this from the pyarrow perspective...any thoughts about this as an acceptable step towards the perfect type stub workflow?

jorisvandenbossche commented 2 months ago

Unfortunately a perfect one doesn't exist for now, I think ;)

The stubtest didn't catch that the stub files were out of sync with the updated code? (I don't expect stubtest to verify if the stubs are correct, but at least that it matches the set of objects that are present in the imported module)

paleolimbot commented 2 months ago

The stubtest didn't catch that the stub files were out of sync with the updated code?

It didn't, and I just made an attempt to intentionally introduce an error in the stubs and found that it wasn't reported either 😬 . I think it has something to do with the fact that all of the files begin with an underscore and maybe aren't type-checked in the same way. VSCode still seems to be able to find the types when dealing with things like array.buffer(0).XXX

paleolimbot commented 2 months ago

I'm going to merge this so that we can have stubs in our release; however, we'll need to follow up to make sure these stay up-to-date! https://github.com/apache/arrow-nanoarrow/issues/624