Skip to content
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 names in __init__.pyi #171

Closed
wants to merge 2 commits into from
Closed

Conversation

tlambert03
Copy link
Contributor

without explicitly exporting names in __init__.pyi, type checkers like mypy won't follow all the names and you won't get autocompletion and code highlighting. I know it's annoying to have to manually do this (and perhaps you will want to automate it if you're autogenerating your type stubs) ... but adding __all__ is important one way or the other

before this PR:

Screenshot 2024-03-04 at 7 27 06 PM Screenshot 2024-03-04 at 7 30 54 PM

after this PR:

Screenshot 2024-03-04 at 7 30 24 PM

@tlambert03
Copy link
Contributor Author

ooof.. looks like stubtest wants __all__ to be in __init__.py as well as __init__.pyi, making it even more annoying. so I added it here

@tlambert03
Copy link
Contributor Author

tlambert03 commented Mar 5, 2024

this actually goes much deeper. As I follow through the docs, I realize that a lot of the star imports from .acquire import * are indeed intended to be used publicly, so you get this sort of thing:

Screenshot 2024-03-04 at 7 48 24 PM

I don't want to sweep in and make any assumptions about the typing strategies here. @nclack, @aliddell, is this something you guys have thoughts on? I know it's annoying to keep static stubs up to date with native code, and it can make for a lot of duplicated stuff without having scripts to autogenerate and test them. is it something you already know about and have opinions on?

@nclack
Copy link
Member

nclack commented Mar 5, 2024

First, we should add type-checking to our examples!

Second, typing this stuff out made me think of a fix! Does #173 work for you @tlambert03?

Anyway, for posterity, here's some answers to questions:

I'm hoping that one day, we won't have to maintain the stubs by hand, but for now, it has to be manual. It's alright as long as we can catch it when it's wrong!

The problem looks to be with re-exporting the types from acquire.pyi. This typechecks (note the acquire.acquire parts):

import acquire

runtime = acquire.acquire.Runtime()
dm = runtime.device_manager()

for device in dm.devices():
	print(device)

props = runtime.get_configuration()
props.video[0].camera.identifier=dm.select(acquire.acquire.DeviceKind.Camera)

The stuff in __init__.py shouldn't really be part of the public API; it's all demo/example code. What we really want is the stuff in acquire.pyi.

@tlambert03
Copy link
Contributor Author

Does #173 work for you @tlambert03?

yeppers

@tlambert03 tlambert03 closed this Mar 5, 2024
@tlambert03 tlambert03 deleted the pyi branch March 5, 2024 02:08
aliddell pushed a commit that referenced this pull request Mar 8, 2024
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