Lazy loading of implementations via entry points

[corranwebster]

PR implements the ideas discussed in #2687 using entry points to contribute widget implementations so that they can be lazily loaded at the point of need, rather than having a monolithic factory module which loads the entire collection of backend implementations at once.

The API is a new get_factory(implementation) function which is intended as a replacement either returns:

• a Factory object which lazily loads attributes via entrypoints in the a group of the form {implementation}.backend.{backend} (where implementation is "toga_core" for now, but could be "togax_..." for projects which want to contribute new widgets)
• a factory module loaded in the old way for backwards compatibility

The factory attribute on a Widget is now a property which defaults to the toga_core factory, but Widget subclasses implementing new widgets should override and instantiate a factory that points at an appropriate togax_* entry point group.

It includes an example of a simple widget and a topic guide on extending. All of this makes it a fairly large addition.

There is a sketch implementation of a moderately complex new widget here https://github.com/corranwebster/toga_bitmap_view. It all seems to work pretty well. The only awkward thing is when adding new entry points you need to do a pip install -e . on the project so it picks up the new metadata.

It should all be backwards compatible to some extent, and so I haven't bumped the version number, but this may be a big enough architectural change to become 0.6.0.

Fixes #2687.

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

[freakboy3742]

This is historical, so it's not of your making... but is there a reason we can't use match in the pytest.raises here?

[freakboy3742]

I guess it doesn't massively matter since it's cached... but is there any reason we can't populate backend as a property of the factory when it is constructed?

[corranwebster]

I've adjusted that and made backend a lazy property (deferring working out what the backend is until it's really needed was the goal).

[freakboy3742]

I know this is still in progress, but I wanted to take a peek to see the direction it was heading - and tl;dr, it looks great.

Three minor comments:

1. Should we add a little structure to the plugin registrations? There's something like 40 of them; breaking them into blocks of similar registrations (similar to how they're grouped in existing factory.__all__ calls might help readability
2. Should we preserve known unimplemented features, commented out? E.g., the textual backend is missing a bunch of features - having them there, but commented out, is one way to communicate a TODO list.
3. We possibly want to test out the API with at least one proof-of-concept "third party" widget before we land this. The BitmapView widget from Yorkshire4 would be a useful test case (especially if we were to break it out as a standalone widget).

[freakboy3742]

Oh - and looking at the test failures... do we want to mark the entire factory module as deprecated? Once this lands, I can't see any reason we'd need to import factory at all, which would meet all the needs of #2547...

[corranwebster]

I know this is still in progress, but I wanted to take a peek to see the direction it was heading - and tl;dr, it looks great.

Three minor comments:

1. Should we add a little structure to the plugin registrations? There's something like 40 of them; breaking them into blocks of similar registrations (similar to how they're grouped in existing factory.__all__ calls might help readability

Yes, this makes sense - didn't do it initially as I wasn't 100% sure about comments in TOML.

2. Should we preserve known unimplemented features, commented out? E.g., the textual backend is missing a bunch of features - having them there, but commented out, is one way to communicate a TODO list.

This also makes sense.

3. We possibly want to test out the API with at least one proof-of-concept "third party" widget before we land this. The BitmapView widget from Yorkshire4 would be a useful test case (especially if we were to break it out as a standalone widget).

I was planning to have an example which implements a simple widget via the new mechanism as a something that we can validate things with, but real use-cases are probably good too.

I've added a first cut at documentation, and had one design change come out of it around how to point a widget's factory at the right entrypoints: I've added a private class var which tells it the argument to use when calling get_factory. It works, but I'm not 100% happy with it.

[corranwebster]

Oh - and looking at the test failures... do we want to mark the entire factory module as deprecated? Once this lands, I can't see any reason we'd need to import factory at all, which would meet all the needs of #2547...

Yes, I was planning to mark these as deprecated via a warning; testing may be a bit of a pain, depending on how things are done.

[johnzhou721]

Oh - and looking at the test failures... do we want to mark the entire factory module as deprecated? Once this lands, I can't see any reason we'd need to import factory at all, which would meet all the needs of #2547...

Yes, I was planning to mark these as deprecated via a warning; testing may be a bit of a pain, depending on how things are done.

Note that the factory is publically documented at Architecture (https://toga.beeware.org/en/latest/topics/architecture/#implementation), so we might want a note there like "Changed in 0.5.5" or something like that.

Also since this affects 3rd-party widgets, should we make a Toga 0.6.0 after we finish top-level navigation so we can advertise this, Canvas refactors, and top-level navigation together in 1 release?

3. We possibly want to test out the API with at least one proof-of-concept "third party" widget before we land this. The BitmapView widget from Yorkshire4 would be a useful test case (especially if we were to break it out as a standalone widget).

Unrelated: That may be due for a conversion to MkDocs.

[freakboy3742]

I've added a first cut at documentation, and had one design change come out of it around how to point a widget's factory at the right entrypoints: I've added a private class var which tells it the argument to use when calling get_factory. It works, but I'm not 100% happy with it.

Hrm - yeah - I see what you mean. I can see the problem you're solving... but I can't help but feel there's a better approach. Maybe defining factory as a cached @property, and require subclasses to override that property definition?

[HalfWhitt]

Also since this affects 3rd-party widgets, should we make a Toga 0.6.0 after we finish top-level navigation so we can advertise this, Canvas refactors, and top-level navigation together in 1 release?

We're holding off on the next version (presumably 0.5.4) until the Canvas refactor is done, or at least has the most important parts in place. That alone, while it does a lot of deprecating, is all localized to one widget that many people won't even be using, so it doesn't warrant a 0.6 bump.

I could maybe see the argument for this PR warranting that, since it's more structural / widely ranging for Toga as a whole. However, we usually reserve version bumps for when we're breaking something, not just adding something.

Also, I very much doubt all three of these will synchronize. The Canvas refactor isn't that far from being essentially done, while it sounds like there's still a lot of design to nail down on navigation. (Not sure of the timeline on this PR.)

[corranwebster]

3. We possibly want to test out the API with at least one proof-of-concept "third party" widget before we land this. The BitmapView widget from Yorkshire4 would be a useful test case (especially if we were to break it out as a standalone widget).

Really hacky and not fully working implementation: https://github.com/corranwebster/toga_bitmap_view

[corranwebster]

I think this is ready for re-review.

[freakboy3742]

This looks great; I've pushed an update with a couple of minor tweaks (mostly to documentation).

The only big issue I found was with the example app. The app works great for me on Cocoa, and on web once I fixed the widget naming issue; it needs a little handholding to get working on Qt, but it did work.

However, it's not clear to me if I'm missing something about the way the app is configured; see the note inline.

Regarding the example bitmap widget - if you want to house that as an official BeeWare project, transfer the repo and we can publish it.

[freakboy3742]

Stray debug, I presume?

Suggested change

print(interface, factory.group, entry_points(group=factory.group))

[freakboy3742]

I'm not sure if I'm missing something here, if there's a bug lurking, or if there's a missed opportunity to provide a better example here.

hello_world_widget currently provides implementations for cocoa, iOS, qt, textual and web. This block of registrations then registers the cocoa, qt and web versions. iOS and Textual are unregistered.

The main app then (re-?)registers the cocoa and qt implementations, using a package name that doesn't appear to exist.

It looks like this is intended to be an example of a custom widget that provides three implementations, plus 2 more implementations provided as part of an app - which is a really good demo. However:

• iOS and Textual implementations aren't ever registered
• the app itself doesn't contain any widget code
• Even if it did, I'm not sure that project.entry-point in the app configuration would register the widgets, as the app's pyproject.toml isn't ever formally "installed" - Briefcase isn't a PEP517 build backend, it just uses PEP518 project configuration.

Am I missing something?

[corranwebster]

It looks like a couple of things went wrong here:

• I'd tried to register at the root level and it didn't work, so I had to split it out but forgot to remove things from the base pyproject.toml. I'll remove the registrations from there since they don't do anything.
• I accidentally committed WIP code for some of the backends

The app not having any widget code is deliberate - the briefcase install process doesn't pick up the entrypoints, so you have to have a separate project ("helloworld" in this case) which has the entrypoints and is a dependency of the project. As you note, I think this is intrinsic to the way that Briefcase works. There's an awkwardness here that may need to be addressed in the docs.

However, for the most part I was running without briefcase, as described in the readme.

I'll clean things up.

[freakboy3742]

I presume you've done this so you can run the Qt testbed app on macOS; if so, I think this should be part of test_sources:

Suggested change

	"../qt/tests_backend",
	]
	]
	test_sources = [
	"../qt/tests_backend",
	]

and removing the corresponding linux test_sources configuration.

[freakboy3742]

This is a historical issue, but it's cleaner as a "match=" clause in the pytest.raises.

[freakboy3742]

Should this be:

Suggested change

	class Label(Widget):
	class HelloWorld(Widget):

?

[corranwebster]

Regarding the example bitmap widget - if you want to house that as an official BeeWare project, transfer the repo and we can publish it.

I'm torn between merging it back in to the yorkshire4 project and having it separate. I do have an actual separate use-case for it myself, which is as a viewer for the microcontroller screen dumps I get from debugging my Tempe project which come out in RGB565 format, but it needs clean-up before I'd be happy with it as official BeeWare.

[corranwebster]

I've cleaned up the example so it should work with briefcase cleanly, and I've filled in all the other backends and have manually tested all of them except winforms, as I don't have a windows vm to test on. Textual is a bit of a hassle: it had difficulty installing a dev version of the textual backend without doing some --no-dep installs and then manually installing things.

Thanks for the fix-up of the other issues.