implement lazy imports

[KRRT7]

• 2547

adds a lazy import mechanism using a module-level getattr function. This applies both to:

1. toga-core (in toga/init.py)
2. toga_cocoa/factory.py)

PR Checklist:

• [ ] All new features have been tested
• [ ] All new features have been documented
• [ ] I have read the CONTRIBUTING.md file
• [ ] I will abide by the code of conduct

[mhsmith]

There are some flake8 errors (find "pre-commit checks" below and click "Details" on the right), mostly like this:

cocoa/src/toga_cocoa/factory.py:58:1: F822 undefined name 'App' in `__all__`.

These can be suppressed by adding a noqa marker:

__all__ = [  # noqa: F822

However, I'm not sure whether toga_cocoa/factory.py even needs an __all__ list anymore, because it's an internal module which should never be used with import *. The list was only needed before to suppress the "unused import" warnings, which won't happen anymore with lazy import.

toga/__init__.py does need an __all__, but I think it should be possible to auto-generate it from the list of lazy import names, as shown in https://github.com/beeware/toga/issues/2547#issuecomment-2091921666. This could be done while generating the "dict which maps from name to module", as mentioned above.

[mhsmith]

The core unit tests (scroll through the checks below and click "Details") are now failing with this error:

../.tox/py-cov/lib/python3.8/site-packages/toga_dummy/widgets/slider.py:6: in <module>
    class Slider(Widget, toga.widgets.slider.SliderImpl):
E   AttributeError: module 'toga.widgets' has no attribute 'slider'

This is a problem with the tests, so it isn't your fault, but you'll still have to fix it before the PR can be merged. The problem is that the tests assume that all the toga submodules have already been imported, which is no longer true now that the imports are lazy.

This can be fixed in that file by changing import toga to import toga.widgets.slider. There are probably a few other places which will have to be updated in the same way. But user code in apps shouldn't be affected, because they should only import the top-level toga module.

To run the tests on your own machine, install Toga into your environment and then follow these instructions.

[KRRT7]

The core unit tests (scroll through the checks below and click "Details") are now failing with this error:

../.tox/py-cov/lib/python3.8/site-packages/toga_dummy/widgets/slider.py:6: in <module>
    class Slider(Widget, toga.widgets.slider.SliderImpl):
E   AttributeError: module 'toga.widgets' has no attribute 'slider'

This is a problem with the tests, so it isn't your fault, but you'll still have to fix it before the PR can be merged. The problem is that the tests assume that all the toga submodules have already been imported, which is no longer true now that the imports are lazy.

This can be fixed in that file by changing import toga to import toga.widgets.slider. There are probably a few other places which will have to be updated in the same way. But user code in apps shouldn't be affected, because they should only import the top-level toga module.

To run the tests on your own machine, install Toga into your environment and then follow these instructions.

all the tests pass on my end.

[freakboy3742]

all the tests pass on my end.

Does this include both the core API tests and the testbed tests? Since the testbed tests are failing on every platform (except iOS) in CI, it suggests you're only running the core API tests locally.

[mhsmith]

why has only the cocoa backend been updated in this way?

Each backend can do this independently, so they don't all need to be done in the same PR. I've removed the "fixes" keyword from the top comment to reflect this.

[freakboy3742]

why has only the cocoa backend been updated in this way?

Each backend can do this independently, so they don't all need to be done in the same PR. I've removed the "fixes" keyword from the top comment to reflect this.

Well... sure... but the logic for each backend will be identical, so I'm not sure why we wouldn't duplicate across all the backends (or, at the very least, the desktop, mobile and dummy backends).

On top of that, via Discord we found out that the one platform the reporter doesn't have access to is macOS, so it seems a weird choice to start there.

[mhsmith]

if we're replacing all those literal imports with programmatic ones, it seems to me that we should be replacing all the factory.py modules with a single Factory class that is instantiated in toga.core.platform, whose __getattr__ implementation does the dynamic submodule lookup

That's a good idea, but I think it can be made even simpler, at the cost of changing all the code that uses the factory. The factory takes a perfectly usable multi-level namespace from the backend package, and creates a new single-level namespace out of it for no significant benefit. What if we instead make the interface layer access the backend module structure directly?

For example, where we currently write:

self.factory.Camera

We would instead have:

import_backend("hardware.camera.Camera")

Which on macOS would translate into an import of toga_cocoa.hardware.camera, and a getattr of Camera.

That way, the names of the backend modules and classes would be distributed throughout the interface layer in the places each of them is used, rather than centralized in one list.

[mhsmith]

Places which use the same name multiple times can be refactored slightly. For example, in Icon.__init__ we currently have self.factory = get_platform_factory() followed by a number of uses of self.factory.Icon. This could be replaced by IconBackend = import_backend("icons.Icon") followed by a number of uses of IconBackend.

[freakboy3742]

That's a good idea, but I think it can be made even simpler, at the cost of changing all the code that uses the factory. The factory takes a perfectly usable multi-level namespace from the backend package, and creates a new single-level namespace out of it for no significant benefit. What if we instead make the interface layer access the backend module structure directly? ... That way, the names of the backend modules and classes would be distributed throughout the interface layer in the places each of them is used, rather than centralized in one list.

I guess that's true: there's no reason to use a single "Factory" namespace, as long as we can clearly specify that path inside the backend module that will contain the class we want to use.

I guess the only question I'd have is whether we include the class name in the import calls (e.g., import_backend('icons.Icon')). If we do the class lookup outside the method, you'd end up with import_backend("icons").Icon - which means import_backend() is a really lightweight wrapper around importlib.import_module() that injects the name of the current backend module - potentially honouring all the argument semantics of import_module as well.

[mhsmith]

If we do the class lookup outside the method, you'd end up with import_backend("icons").Icon - which means import_backend() is a really lightweight wrapper around importlib.import_module() that injects the name of the current backend module

Yes, that would be a bit cleaner. And in most cases, each interface module only uses one backend module, so we could adopt the pattern of calling it backend and putting it at the module level below the import statements:

backend = import_backend("hardware.camera")

and then self.factory.Camera becomes backend.Camera.

potentially honouring all the argument semantics of import_module as well

The only other argument of import_module is package, and it wouldn't make any sense to accept that because the package should always be the same. Neither would it make sense to require relative module syntax, because the number of leading dots could never be anything other than 1.

[freakboy3742]

If we do the class lookup outside the method, you'd end up with import_backend("icons").Icon - which means import_backend() is a really lightweight wrapper around importlib.import_module() that injects the name of the current backend module

Yes, that would be a bit cleaner. And in most cases, each interface module only uses one backend module, so we could adopt the pattern of calling it backend and putting it at the module level below the import statements:

backend = import_backend("hardware.camera")

and then self.factory.Camera becomes backend.Camera.

Agreed this would be nice, provided there's no circular import or other dependency issues lurking.

potentially honouring all the argument semantics of import_module as well

The only other argument of import_module is package, and it wouldn't make any sense to accept that because the package should always be the same. Neither would it make sense to require relative module syntax, because the number of leading dots could never be anything other than 1. 👍

[mhsmith]

In that case, how about we reduce this PR to just doing lazy imports in the core, and record a separate issue for the factory change ideas?

[freakboy3742]

In that case, how about we reduce this PR to just doing lazy imports in the core, and record a separate issue for the factory change ideas?

I'd be OK with that - but either way, this PR will need to be modified (either adding the new imports, or removing the dynamic import implementation on the platform backends being used here); which way we go depends on @KRRT7's interest level.

[mhsmith]

Agreed this would be nice, provided there's no circular import or other dependency issues lurking.

Good point: there are a few places where the backend needs to access things from the frontend at module level, like toga.widgets.slider.SliderImpl. These should be reviewed before committing to an approach.

[KRRT7]

reduce this PR to just doing lazy imports in the core

let's do this, and let's open the factory change ideas and once you guys have come to a decision on what to do, i'll give it a look and discuss how best to implement it.

[mhsmith]

Continued at:

• 2686

• 2687