-
Notifications
You must be signed in to change notification settings - Fork 48
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Export type annotations for public use #95
Conversation
Preparatory for next commit.
If we want to export our type information, we're required to wrap our modules in a package. Do so, but leave compatbility wrappers in the old locations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR!
Had a quick look, lgtm - but I am not that familiar with type annotations and cython.
@Nikratio can you have a look?
Thanks for the quick review! Marked as draft; I have a few more changes coming. |
Use string annotations to work around unparameterizable asyncio types in Python 3.8. Also replace one use of set[] with Set[], again for Python 3.8.
__cinit__() is an implementation detail; __init__() is what callers see.
We were also importing async_wrapper without referencing or re-exporting it. It's not clear that it's useful outside the pyfuse3 implementation, so for now, drop the import rather than re-exporting.
Okay, this should be ready for review. CI should pass now. This update fixes some additional type issues, adds a new I've gone through pyfuse3-stubs and compared its types to the ones in pyfuse3, incorporating any fixes I found. pyfuse3-stubs includes additional types for some pyfuse3 internals like cc: |
The application may want to use them at runtime, e.g. with typing.cast().
This tells the typechecker to complain if the application tries to assign to these attributes.
Similar to FileNameT.
It makes the logs a bit easier to read. Preparatory for next commit. Drop redundant set -e; GitHub Actions configures it automatically.
Sphinx autoclass is currently confused by NewTypes, so document those by hand. Also, Sphinx wants to document the NewTypes as coming from pyfuse3._pyfuse3, and there doesn't seem to be a setting or rST directive to convince it otherwise. Work around this by mangling the types' __module__ attributes from conf.py.
Updated once again, to fix docs and to re-export the NewTypes at runtime for use e.g. with |
@Nikratio, per #95 (review), is there any chance you could take a look? |
Sorry for the delay. It seems that |
It's probably because there's still a |
Thanks for the reviews! |
Do you have a sense of when this might end up in a release? |
@bgilbert there is only 1 small issue left to do in the 3.3.1 milestone, so guess I could do a release in the near future. |
Type annotations are currently internal to pyfuse3, so applications can't use them for their own type checking. Fix some bugs and omissions in the annotations and make them public by adding a
py.typed
file.To do so, we're required to move our modules into a Python package. Do this, but add forwarders for the
_pyfuse3
andpyfuse3_asyncio
modules to avoid breaking compatibility.Probably best reviewed as individual commits. GitHub's rendering of the overall diff is somewhat unhelpful.