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

Refactor initialization and re-order methods #11

Merged
merged 5 commits into from
Nov 11, 2024
Merged

Conversation

almarklein
Copy link
Member

@almarklein almarklein commented Nov 7, 2024

Tasks

  • Refactors the initialization so implementing classes become simpler.
  • Use double-underscore where we can.
  • Prefix all methods that subclasses must implement with _rc_.
  • In the backends, put methods in a consistent order.
  • Test for glfw
  • Test for qt
  • Test for offscreen
  • Test for wx
  • Test for jupyter

Decisions

Some notes on decisions I came to. I feel like the text below makes little sense for anyone else, since its all a bit hard to explain and the problems are rather subtle. Nevertheless, I think its good to have this on record.

Init order

The initialization order is tricky. Taking qt as an example, the mro is:

  • QObject
  • ...
  • QWidget
  • BaseRenderCanvas
  • QRenderWidget

All is good, except for one thing: I want BaseRenderCanvas to set the size and title (and maybe more in the future), because I don't want every subclass to have to bother with that. However, that must happen after the QRenderWidget did its initialization. I played with some ideas, and implemented one where the the backend class (QRenderWidget) must implement _rc_init instead of __init__. But this is too weird. Thought of metaclasses, but that's too dark. Using __new__ is also no help.

So I ended up with classic instantiation mechanics, and requiring backend canvas classes to call _final_canvas_init() to allow BaseRenderCanvas to apply the final configuration (setting size, title, transparency etc.) A little anoying, but its explicit. And if the class author forgets to call it, you simply don't get the custom title and size set.

Subwidget kwargs

Another tricky one, specific to the wrapper classes in qt and wx, was how to pass the right kwargs to the real canvas (._subwidget). I previously had tricks like a function that popped canvas kwargs form the kwargs dict. In the end the solution is quite simple: any kwargs that the toplevel widget wants, must be explicitly taken, the rest is passed to the subwidget:

class TopLevelClass(WrapperRenderCanvas, QWidget):
    def __init__(self, parent=None, **kwargs):
        super().__init__(parent)
        self._subwidget = QRenderWidget(self, **kwargs)

@almarklein
Copy link
Member Author

Added some notes in the top comment.

@almarklein almarklein marked this pull request as ready for review November 8, 2024 23:13
@almarklein almarklein merged commit 95bb8f4 into main Nov 11, 2024
11 checks passed
@almarklein almarklein deleted the refactor-reorder branch November 11, 2024 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant