Skip to content

export symbols using __all__ to avoid typing errors #1371

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

Merged
merged 4 commits into from
May 7, 2025

Conversation

Andrej730
Copy link
Contributor

Mentioned in #1322

Explanation on how it works - https://github.com/microsoft/pyright/blob/main/docs/typed-libraries.md#library-interface

Example failing snippet:

import pygit2

# "Repository" is not exported from module "pygit2" PylancereportPrivateImportUsage
def test(repo: pygit2.Repository):
    ...

Wanted to solve that particular error with missing Repository symbol and I'm not very familar with pygit2 repository - so please advice if any symbols should be removed or added.
Currently I've included all symbols that are imported in __init__, except ffi, C (looked like something private), to make it fully compatible with current from pygit2 import * behaviour (definingi __all__ is limiting wild card import).

@jdavid
Copy link
Member

jdavid commented May 5, 2025

import sys
before = set(dir())
from pygit2 import *
after = set(dir())
imported_symbols = after - before
print(len(imported_symbols))

This prints 412, while in this PR __all__ has 127 symbols.
For backwards compatibility you can just put all of them, sorted alphabetically, then I will go through and remove those deemed internal or deprecated.

Also put __all__ at the very end of the file.

@Andrej730
Copy link
Contributor Author

That's a lot missed symbols! Submitted new commits - first one just added all symbols in A-z order, second one categorized them so it will be easier to discard some specific cateogires and then maintain it.

This is how I was testing it:

Test symbols without __all__ taking effect (__all__ renamed to __all__test):

before = set(dir())
from pygit2 import *

after = set(dir())
imported_symbols = after - before - {"before"}

import pygit2
all_symbols = set(pygit2.__all__test)
print(len(all_symbols)) # 411
print(len(imported_symbols)) # 411
assert len(diff:=(all_symbols - imported_symbols)) == 0, diff
assert len(diff:=(imported_symbols - all_symbols)) == 0, diff

Test with __all__ enabled:

before = set(dir())
from pygit2 import *

after = set(dir())
imported_symbols = after - before - {"before"}

import pygit2
assert len(imported_symbols) == len(set(pygit2.__all__)) == 411
assert set(imported_symbols) == set(pygit2.__all__)

@jdavid jdavid merged commit 14c60e1 into libgit2:master May 7, 2025
8 checks passed
@Andrej730 Andrej730 deleted the define-all branch May 7, 2025 16:54
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