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

Export hardware implementation abstract base classes and protocols #3022

Closed
sarayourfriend opened this issue Dec 5, 2024 · 4 comments
Closed
Labels
enhancement New features, or improvements to existing features.

Comments

@sarayourfriend
Copy link
Contributor

sarayourfriend commented Dec 5, 2024

What is the problem or limitation you are having?

In working on #2999 I had to do a fair bit of software archeology to uncover the interface required of the platform-specific implementations of Location. For location, this includes implementing a handful of required methods, as well as understanding the shape of the interface constructor argument (primarily to find the change-handler). Additionally, there are some small conventions that each implementation is expected to follow (e.g., in the case of Location, the use of a native attribute to store the platform's actual Location API). In particular, I also initially had a bit of confusion wrapping my head around what was meant to be a property vs a method, in particular when that implementation detail differed between the core wrapper and the platform implementation (e.g., with has_permission).

In general, this was not too burdensome, but better documentation would establish expectations and requirements more clearly than reading other implementations, each of which have their own "quirks" that add noise related to the target platform. It should speed initial development of new implementations of features for existing platforms, and especially help speed implementation of entirely new platforms (as in those cases, it's helpful to have those expectations up front to understand the scope).

Describe the solution you'd like

To use Location as an example, an ABC PlatformLocation could be defined in Toga core outlining the methods required of a complete implementation.

I've drafted a PR to demonstrate the general idea: #3023. See the PR description for a few more details.

Describe alternatives you've considered

Write prose documentation outlining the requirements. These tend to drift, though, and an ABC actually provides an assurance that a platform's implementation has the methods, along with useful type-hints for devs of both Toga core and individual platforms

Additional context

No response

@sarayourfriend sarayourfriend added the enhancement New features, or improvements to existing features. label Dec 5, 2024
@sarayourfriend
Copy link
Contributor Author

sarayourfriend commented Dec 5, 2024

In fact, this doesn't even necessarily need to be scoped just to hardware. I hadn't looked at widget implementations very closely before, but of course they follow a similar pattern of core wrapping a platform implementation, and often having similar potentially confusing and subtle differences that would be easy to forget when writing the platform impl (e.g., toga.Widget.window is a settable property, whereas the platform implementation should instead define a set_window method).

If the general idea works out for hardware, similar ABCs could be created for widgets and other things following this pattern.

@freakboy3742
Copy link
Member

So - I see what you're aiming for here; my question is whether the juice is worth the squeeze.

I completely acknowledge that you've hit the worse case here. The API contract between Toga's interface and implementation definitely isn't obvious, nor is it well documented. Someone (like yourself) that is implementing a missing piece of an API doesn't have a clear set of instructions to follow. And yes - the use case you've found for hardware APIs also exists for widgets, and anything else represented in the backend.

FWIW, the closest that you'll get right now to what you're looking for is probably the dummy interface - broadly speaking, that interface will define the simplest possible compliant implementation of any given API. That won't entirely capture the semantics of the API... but then, neither does an ABC or Protocol definition.

But - once an implementation exists, having an ABC or Protocol definition won't actually help that much. There's a very small amount of common code (such as setting self.interface and the like), but for the most part, an ABC only serves to document the methods that are required. Now that the GTK backend has an implementation of Location... those methods are all implemented, and GTK isn't going to need another implementation.

If this were a public-visible API that we were expecting end-users to implement, it might be a different story, but there's a pretty limited scope for usage for the impl of this API. You'll only need to use this sort of ABC if you're writing your own backend - so the writer of toga-qt might need it... maybe the author of toga-winui3 (once Microsoft gets that UI story straight)... but there honestly aren't that many others on the horizon that I can think of.

However, on the other side of the coin - every byte of code in an interface is something that needs to be read, parsed, compiled and interpreted; and every subclass relationship adds a little bit to the memory footprint and class instantiation time - that's not a huge imposition, especially on desktop platforms; but on mobile, and especially on web, every byte counts. If startup time is being slowed by providing a formal definition of an interface that isn't really being used in practice, then I'm not convinced adding formal interfaces like this is such a good bargain.

If we were going to pursue this, I'd be inclined to go down the documentation path - providing an "backend implementers guide" as part of technical reference documentation. That would give us a place to put API definitions, but also broader "this is how it's meant to work" and other quirks and expectations of the backend implementing an API.

@sarayourfriend
Copy link
Contributor Author

Point taken re: size, and that the experience of writing an implementation is uncommon, so the trade-off is bad.

If we were going to pursue this, I'd be inclined to go down the documentation path - providing an "backend implementers guide" as part of technical reference documentation. That would give us a place to put API definitions, but also broader "this is how it's meant to work" and other quirks and expectations of the backend implementing an API.

That sounds good. I'd definitely like to work on this, even for my own benefit, it would probably help to make sure my takeaways from working on #2999 are sound. If not for the Toga documentation, I'd at least document it for myself 🙂

If you'd like to close this as "won't do" and open a new issue for adding an "implementers guide", or if I should just convert this issue, let me know, and I'm happy to do the tidying on it.

@freakboy3742
Copy link
Member

Let's close this and open a new issue for the "implementers guide". Thanks for the suggestion though - it's always helpful to have someone challenge the current state of things, even if the idea that is proposed isn't ultimately pursued.

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

2 participants