-
Notifications
You must be signed in to change notification settings - Fork 3
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
REF: create a window singleton for access throughout superscore #115
base: master
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
115 ref_window_singleton | ||
######################## | ||
|
||
API Breaks | ||
---------- | ||
- N/A | ||
|
||
Features | ||
-------- | ||
- N/A | ||
|
||
Bugfixes | ||
-------- | ||
- N/A | ||
|
||
Maintenance | ||
----------- | ||
- Makes Window a QtSingleton, and adds an access function for use throughout superscore ui widgets | ||
Uses this singleton to access methods that were passed around manually in the past as optional arguments or set attributes (open_page_slot, client) | ||
|
||
Contributors | ||
------------ | ||
- tangkong |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,15 +4,17 @@ | |
from __future__ import annotations | ||
|
||
from pathlib import Path | ||
from typing import ClassVar, List | ||
from typing import ClassVar, List, Optional | ||
from weakref import WeakValueDictionary | ||
|
||
from pcdsutils.qt.designer_display import DesignerDisplay | ||
from qtpy import QtCore, QtWidgets | ||
|
||
from superscore.client import Client | ||
from superscore.qt_helpers import QDataclassBridge, QDataclassList | ||
from superscore.type_hints import AnyDataclass | ||
from superscore.type_hints import AnyDataclass, OpenPageSlot | ||
from superscore.utils import SUPERSCORE_SOURCE_PATH | ||
from superscore.widgets import get_window | ||
from superscore.widgets.manip_helpers import (FrameOnEditFilter, | ||
match_line_edit_text_width) | ||
|
||
|
@@ -423,3 +425,43 @@ def __call__(cls, *args, **kwargs): | |
if cls._instance is None: | ||
cls._instance = super().__call__(*args, **kwargs) | ||
return cls._instance | ||
|
||
|
||
class WindowLinker: | ||
""" | ||
Mixin class that provides access methods for resources held by the main Window. | ||
These include: | ||
- client: first attempts to grab the client set at init, if none exists use | ||
the Window's client | ||
- open_page_slot: grabs the slot from the Window | ||
""" | ||
|
||
def __init__(self, *args, client: Optional[Client] = None, **kwargs) -> None: | ||
self._client = client | ||
super().__init__(*args, **kwargs) | ||
|
||
@property | ||
def client(self) -> Optional[Client]: | ||
# Return the provided client if it exists, grab the Window's otherwise | ||
if self._client is not None: | ||
return self._client | ||
else: | ||
window = get_window() | ||
if window is not None: | ||
return window.client | ||
|
||
@client.setter | ||
def client(self, client: Client): | ||
if not isinstance(client, Client): | ||
raise TypeError(f"Cannot set a {type(client)} as a client") | ||
|
||
if client is self._client: | ||
return | ||
|
||
self._client = client | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I notice that this allows different widgets to use different clients. Is this intended? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Within the context of the superscore app, this isn't something we expect to happen. We should only have one client for one superscore instance and all its child pages. I'm not particularly worried about a new client getting slipped into the superscore app, but this does provide that possibility Personally, specifying this per widget lets me test widgets individually, free of needing to create a |
||
|
||
@property | ||
def open_page_slot(self) -> Optional[OpenPageSlot]: | ||
window = get_window() | ||
if window is not None: | ||
return window.open_page |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test design question: when would you choose
unittest.mock.patch
over the pytestmonkeypatch
fixture and vice versa?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there's not much of a difference most of the time. In this case I wanted to mock a property, and the first example I found was through unittest.mock. I've been using
MagicMock
and finding it very convenient, but it is some deep mystical magicI should probably be better about sticking to one, but we have access to both.