beeware
implement lazy loading in toga_core
- Fixes (partly) #2547
adds a lazy import mechanism using a module-level getattr function. This applies to:
- toga-core (in toga/init.py) Only
PR Checklist:
- [ X] All new features have been tested
- [ X] All new features have been documented
- [X ] I have read the CONTRIBUTING.md file
- [ X] I will abide by the code of conduct
this comes behind #2584 , it was very messy and would require lots of reverts, please see there for history and revelance.
The core of this looks good; a couple of minor cleanups flagged inline.
Thanks, the issues were resolved
This all looks good to me, but even performance improvements should have a test where possible. Since the tests are run in alphabetical order, the app tests are run first, so we could put a test in there which does the following:
- Pick a widget module that isn't used elsewhere in the
apptests, e.g.toga.widgets.button. - Assert that the module doesn't exist in
sys.modules. - Access
toga.Button. - Assert that the module now exists in
sys.modules, and thattoga.Button is toga.widgets.button.Button.
I'd be a little wary of adding a test that depends on execution order - while default invocation of pytest will execute in alphabetical order, there's no guarantee tests will be executed in that order (especially if they're manually specified).
An explicit test that a widget such as Button has been imported correctly (whether by the test, or by running a Button test previously) is definitely called for - but I don't think the "from a clean state" requirement (i.e., that sys.modules was clean beforehand) is needed.
There should still be a test verifying that a clean import of toga doesn't import all the widgets. Otherwise, if that somehow started happening again in the future, we might not notice for a long time.
The test itself could create the clean slate by removing toga and toga.widgets.button from sys.modules, and restoring them afterwards. Then it wouldn't matter which order it ran in, and it could be placed somewhere more appropriate than the app directory.
The test itself could create the clean slate by removing
togaandtoga.widgets.buttonfromsys.modules, and restoring them afterwards. Then it wouldn't matter which order it ran in, and it could be placed somewhere more appropriate than theappdirectory.
True - although there's also the implementation cache of globals() that needs to be cleared.
That said - looking at the current implementation, the value is being set in globals(), but that cached value is never being used... as written, every import will fire the import machinery, and the value in globals() will never be used.
True - although there's also the implementation cache of
globals()that needs to be cleared.
Every module object has its own globals, so a freshly-imported toga module will have nothing cached.
That said - looking at the current implementation, the value is being set in
globals(), but that cached value is never being used
See "customizing module attribute access" – adding the value to globals allows it to be found "through the normal lookup", so __getattr__ will not be called with that name again.
See "customizing module attribute access" – adding the value to
globalsallows it to be found "through the normal lookup", so__getattr__will not be called with that name again.
Huh - my mental model has __getattr__ responsible for all module attribute lookup if it existed. If it's only used as a supplement to what is found in globals(), then I agree this isn't an issue.