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

Eliminate factory modules #2687

Open
mhsmith opened this issue Jul 1, 2024 · 7 comments
Open

Eliminate factory modules #2687

mhsmith opened this issue Jul 1, 2024 · 7 comments
Labels
enhancement New features, or improvements to existing features.

Comments

@mhsmith
Copy link
Member

mhsmith commented Jul 1, 2024

The sub-module structure of each backend is identical, so that can already be considered a well-defined API between the interface and backend layers. Putting the "factory" namespace in between them adds complexity for no benefit, and also forces all the sub-modules to be imported whether the app uses them or not (#2547).

For example, where we currently write:

self.factory.Camera

We could 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.

For previous discussion, start at #2584 (review).

@mhsmith mhsmith added the enhancement New features, or improvements to existing features. label Jul 1, 2024
@mhsmith mhsmith mentioned this issue Jul 1, 2024
4 tasks
@freakboy3742
Copy link
Member

As noted in this comment, it may be preferable to expose this API as import_backend("hardware").camera.Camera or import_backend("hardware.camera").Camera; that makes import_backend a relatively lightweight wrapper around import_module, but injecting the platform name, and yielding a module.

A related thought: this mechanism could also be used to provide a plugin interface for third-party widgets to provide platform-specific implementations. Consider the case of a third party widget that has native platform implementations - the widget could register as having providing a macOS implementation, then the interface use a standard import_backend API to access the platform backend. That avoids third-party widget needing to replicate the "if sys.platform ..." logic that would be the core of import_backend, and should be able to do this as a fallback if an AttributeError is raised on a first-pass lookup on the built-in backend.

@HalfWhitt
Copy link
Contributor

HalfWhitt commented Dec 3, 2024

I've been looking into this (and #2928), and I've noticed an inconsistency.

Remember that _IMPL_NAME property I stuck on widget classes in my first pass on widget initialization in #2942, as a way of centralizing the implementation-finding logic, yet still keeping the same implementation if a user subclasses an existing Toga widget? Window uses essentially the same thing with its _WINDOW_CLASS attribute. And StatusIcon uses the same sort of dynamic lookup to get its implementation, but simply uses the current object's class name... which would break a user-made subclass.

I'm not entirely sure yet what, if anything, I want to do about either of those, but it seems they could be part of whatever this implementation-fetching redesign ultimately looks like.

I share your suspicion that this and #2928 are closely linked. Since adding them will change how users can create and subclass custom widgets — which #2942 already does too — perhaps it would be best to get these in before 0.5.0, so there's only one churn instead of two.

@freakboy3742
Copy link
Member

I'm not entirely sure yet what, if anything, I want to do about either of those, but it seems they could be part of whatever this implementation-fetching redesign ultimately looks like.

Agreed - my initial impression would be that using the _create() approach from #2942 would at least be consistent.

I share your suspicion that this and #2928 are closely linked. Since adding them will change how users can create and subclass custom widgets — which #2942 already does too — perhaps it would be best to get these in before 0.5.0, so there's only one churn instead of two.

Agreed. The combination with this ticket also highlights that although #2928 is framed in terms of "widgets", there's a need to register other types of objects - windows, icons, hardware, and potentially more.

@HalfWhitt
Copy link
Contributor

Agreed - my initial impression would be that using the _create() approach from #2942 would at least be consistent.

Sounds reasonable. Even if things ultimately become something different, at least doing that as a first pass means it'll be easier to change everything the same way.

Another thing, too, as I ponder and sketch out how all this fits together — in what form do we expect people to be creating and providing Toga additions? Toga itself is a separate distribution package (and import package) for core and for each backend. That's great for flexibility and only installing what you need, but it's also pretty complex as a minimum requirement for someone to start trying out their custom widget, especially if they only have one or two backends implemented. If the registration mechanism allows one to specify the root of each backend, then someone could provide separate packages or simple submodules inside their package.

Also... It would go against the precedent set with image plugins and backends themselves, but I keep wondering if it might be worth using a registration mechanism along the lines of what Python does with ABCs — you create your own sequence type and then do Sequence.register(my_type). It's explicit in the code itself, and it can be done / tested out even in code that isn't part of an installed package and thus lacks entry points.

Agreed. The combination with this ticket also highlights that although #2928 is framed in terms of "widgets", there's a need to register other types of objects - windows, icons, hardware, and potentially more.

I suspect most subclassing of Window will be to customize or add behavior at the core level, but I suppose you never know... I think widgets and hardware should probably be the priority, as far as registering custom implementations goes.

@freakboy3742
Copy link
Member

Another thing, too, as I ponder and sketch out how all this fits together — in what form do we expect people to be creating and providing Toga additions?

Good question - I don't know for sure. A single installable package is easy to set up and explain, but complicates the process of defining and installing dependencies. Multiple installable packages (mirroring the Toga repo setup) is the most robust, but a lot harder to set up. We may need to provide template repositories for bootstrapping new widgets.

Also... It would go against the precedent set with image plugins and backends themselves, but I keep wondering if it might be worth using a registration mechanism along the lines of what Python does with ABCs — you create your own sequence type and then do Sequence.register(my_type). It's explicit in the code itself, and it can be done / tested out even in code that isn't part of an installed package and thus lacks entry points.

AFAIK, that's mostly an accident of history. The metadata associated with extras and plugins has really only been well formalised since 3.8 or thereabouts; so a lot of patterns in the CPython codebase are born out of necessity because the metadata couldn't be relied on.

@HalfWhitt
Copy link
Contributor

A single installable package is easy to set up and explain, but complicates the process of defining and installing dependencies. Multiple installable packages (mirroring the Toga repo setup) is the most robust, but a lot harder to set up. We may need to provide template repositories for bootstrapping new widgets.

Perhaps even a script that generates a skeleton, à la what what Briefcase can do for a new project.

AFAIK, that's mostly an accident of history. The metadata associated with extras and plugins has really only been well formalised since 3.8 or thereabouts; so a lot of patterns in the CPython codebase are born out of necessity because the metadata couldn't be relied on.

Huh, I hadn't put two and two together there. I still don't really have entry points well stored in my head — despite having added them for the image plugins — but it's probably just because they're still new to me.


Oh, hey. @t-arn , just giving you a heads up about all this. With this issue, #2928 (third-party widget registration), and #2942 (re-ordered widget initialization), the changes for third-party widget definition in 0.5.0 are still in flux, but are probably going to affect your taTogaLib.

@t-arn
Copy link
Contributor

t-arn commented Dec 3, 2024

@HalfWhitt Thanks for the warning. It's always good to know when breaking changes are around the bend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New features, or improvements to existing features.
Projects
None yet
Development

No branches or pull requests

4 participants