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

Let stub-defined __all__ override imports #133

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lagru
Copy link
Member

@lagru lagru commented Nov 27, 2024

In scikit-image, I encountered the case where I want to make certain functions available for lazy-loading while at the same time not wanting to advertise their existence via __all__ (see scikit-image/scikit-image#6892). To implement this with the current behavior of lazy_loader.attach_stub I had to specify __all__ twice, both in the PY and in the PYI file (see scikit-image/scikit-image@40157d0).

Ideally, I'd like to avoid this duplication. So I came up with a quick "work in progress" solution, in order to discuss this use case here. The current draft has some drawbacks for now, e.g. it makes certain assumptions about how __all__ must be defined in the stub. I'm happy to iterate on this, depending on which kind of behavior is wanted.

Please, let me know what you think. 😊

Copy link

codecov bot commented Nov 27, 2024

Codecov Report

Attention: Patch coverage is 78.78788% with 7 lines in your changes missing coverage. Please review.

Project coverage is 93.63%. Comparing base (4e78314) to head (9579004).

Files with missing lines Patch % Lines
lazy_loader/__init__.py 70.83% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #133      +/-   ##
==========================================
- Coverage   95.74%   93.63%   -2.12%     
==========================================
  Files           5        5              
  Lines         235      267      +32     
==========================================
+ Hits          225      250      +25     
- Misses         10       17       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

This enables the use case where objects should be available for
lazy-loading while not advertising them. This might be useful for
deprecations.
@stefanv
Copy link
Member

stefanv commented Dec 5, 2024

__all__ is really only used when doing from X import * isn't it? So, mostly I think __dir__ is what we care about. Then there are two options: populate __dir__ as everything that is lazy loaded, or take into consideration __all__.

Could that be a simpler approach, here, simply adding a use_all flag that changes both what comes back in __all__ and in __dir__?

@lagru
Copy link
Member Author

lagru commented Dec 6, 2024

all is really only used when doing from X import * isn't it?

Probably? There might be tools out there to find the public API that way. Can't remember how Sphinx does it, but probably with dir()?

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