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.

Sign up for GitHub

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

fix (GT-96): parameters tab search past first #15

Merged
merged 8 commits into from
Apr 2, 2021
Merged

fix (GT-96): parameters tab search past first #15

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d9c87b8
fix (GT-96): parameters tab search past first
gordon-epc Mar 30, 2021
8949407
fix (GT-96): add typing to modified classes, add base class
gordon-epc Mar 30, 2021
f542afb
fix (GT-96): update to appease black
gordon-epc Mar 30, 2021
e505610
fix (GT-96): columns attr.ib definition with converter
gordon-epc Mar 31, 2021
821d72c
fix (GT-96): search_view better index
gordon-epc Mar 31, 2021
be4146c
fix (GT-96): message_handler prints stack trace
gordon-epc Mar 31, 2021
64e4243
fix (GT-96): once again, update to appease black
gordon-epc Mar 31, 2021
a993444
fix (GT-96): undo inheritance for proxy models
gordon-epc Apr 1, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 9 additions & 14 deletions epyqlib/device.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -720,7 +720,7 @@ def _init_from_parameters(
column = epyqlib.txrx.Columns.indexes.name
for view, model in pairs:
if model.root.tx:
proxy = epyqlib.utils.qt.PySortFilterProxyModel(
proxy = epyqlib.utils.qt.SortFilterProxyModel(
filter_column=column,
)
proxy.setSortCaseSensitivity(Qt.CaseInsensitive)
Expand Down Expand Up @@ -851,13 +851,7 @@ def none_or_uuid(uuid_string):
path=self.nvs.access_level_node.signal_path(),
)

sort_proxy = epyqlib.utils.qt.PySortFilterProxyModel(
filter_column=column,
)
sort_proxy.setSortCaseSensitivity(Qt.CaseInsensitive)
sort_proxy.setSourceModel(nv_model)

diff_proxy = epyqlib.utils.qt.DiffProxyModel(
proxy = epyqlib.utils.qt.HighlightDiffSortFilterProxyModel(
columns=epyqlib.nv.diffable_columns,
reference_column=(epyqlib.nv.Columns.indexes.user_default),
diff_highlights={
Expand All @@ -866,12 +860,13 @@ def none_or_uuid(uuid_string):
reference_highlights={
QtCore.Qt.ItemDataRole.BackgroundRole: epyqlib.nv.reference_highlight,
},
filter_column=column,
)
diff_proxy.setSourceModel(sort_proxy)

view.setModel(diff_proxy)
view.configure_sort_proxy(sort_proxy)
view.configure_diff_proxy(diff_proxy)
proxy.setSourceModel(nv_model)
proxy.setSortCaseSensitivity(Qt.CaseInsensitive)
view.setModel(proxy)
view.configure_sort_proxy(proxy)
view.configure_diff_proxy(proxy)

view.set_metas(self.metas)
view.set_sorting_enabled(True)
Expand All @@ -896,7 +891,7 @@ def none_or_uuid(uuid_string):
)

column = epyqlib.variableselectionmodel.Columns.indexes.name
proxy = epyqlib.utils.qt.PySortFilterProxyModel(
proxy = epyqlib.utils.qt.SortFilterProxyModel(
filter_column=column,
)
proxy.setSortCaseSensitivity(Qt.CaseInsensitive)
Expand Down
2 changes: 1 addition & 1 deletion epyqlib/tests/pm/test_parametermodel.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ def test_search_in_column(sample, column, target):
),
)
def test_proxy_search_in_column(sample, column, target):
proxy = epyqlib.utils.qt.PySortFilterProxyModel(filter_column=0)
proxy = epyqlib.utils.qt.SortFilterProxyModel(filter_column=0)
proxy.setSourceModel(sample.model.model)

(index,) = proxy.match(
Expand Down
2 changes: 1 addition & 1 deletion epyqlib/tests/test_attrsmodel.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ def test_proxy_search_column_2(qtbot):
def proxy_search_in_column(column, target):
model = make_a_model()

proxy = epyqlib.utils.qt.PySortFilterProxyModel(filter_column=0)
proxy = epyqlib.utils.qt.SortFilterProxyModel(filter_column=0)
proxy.setSourceModel(model.model)

view = PyQt5.QtWidgets.QTreeView()
Expand Down
2 changes: 1 addition & 1 deletion epyqlib/tests/utils/test_qt.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ def diff_proxy_test_model():
for column in range(columns):
model.setItem(row, column, PyQt5.QtGui.QStandardItem())

proxy = epyqlib.utils.qt.DiffProxyModel(
proxy = epyqlib.utils.qt.HighlightDiffSortFilterProxyModel(
columns=range(1, rows),
diff_highlights={
PyQt5.QtCore.Qt.ItemDataRole.BackgroundRole: (PyQt5.QtGui.QColor("orange"))
Expand Down
62 changes: 39 additions & 23 deletions epyqlib/utils/qt.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import textwrap
import time
import traceback
import typing
import uuid
import weakref

Expand Down Expand Up @@ -153,6 +154,10 @@ def message_handler(mode, context, message):
)
)
print(" {}: {}\n".format(mode, message))
# Output stack track, minus the last entry which unnecesarily points to the next line.
tb_stack = traceback.extract_stack()
tb_format = traceback.format_list(tb_stack[:-1])
print("".join(tb_format))


class Progress(QtCore.QObject):
Expand Down Expand Up @@ -631,14 +636,21 @@ def dialog_from_file(parent, title, file_name):
)


class PySortFilterProxyModel(QtCore.QSortFilterProxyModel):
def __init__(self, *args, filter_column, **kwargs):
super().__init__(*args, **kwargs)
@attr.s(auto_attribs=True)
class BaseSortFilterProxyModel(QtCore.QSortFilterProxyModel):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had imagined this being a generic QObject inheriting thing but I kind of forgot about how we want to inherit from something else... I guess maybe it would still work and then the other class would just get mixed in?

@attr.s(auto_attribs=True)
class AttrsQObject(QtCore.QObject):
    _parent: QtCore.QObject = None

    def __attrs_post_init__(self):
        super().__init__(self._parent)

@attr.s(auto_attribs=True)
class SortFilterProxyModel(AttrsQObject, QtCore.QSortFilterProxyModel):

This is really a big picture question though and I can't say I'm hot on the multiple inheritance I just typed here... maybe it isn't worth it and a pattern is the right approach.

_parent: QtCore.QObject = None

def __attrs_post_init__(self):
super().__init__(self._parent)

# TODO: replace with filterKeyColumn
self.filter_column = filter_column

self.wildcard = QtCore.QRegExp()
@attr.s(auto_attribs=True)
class SortFilterProxyModel(BaseSortFilterProxyModel):
filter_column: int = 0
wildcard: QtCore.QRegExp = attr.Factory(QtCore.QRegExp)

def __attrs_post_init__(self):
super().__attrs_post_init__()
self.wildcard.setPatternSyntax(QtCore.QRegExp.Wildcard)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(tl;dr probably do nothing, but I felt like thinking about this so I wrote this up anyways)

So, I just learned that you can pass properties to the constructors with PyQt (python-qt-tools/PyQt5-stubs#147 (comment)). I don't know that I want to use that, or if it works with PySide (maybe relevant someday here, definitely relevant elsewhere for us), but figured I'd share anyways. Point is that this could get moved up to the attribute definition then.

wildcard: QtCore.QRegexp = attr.Factory(lambda: QtCore.QRegexp(patternSyntax=QtCore.QRegexp.Wildcard)

But that still doesn't get rid of the .__attrs_post_init__() so... whatever... until we write a qt-specific attrs decorator... someday. Or maybe your change below leads somewhere good. Taking it a step further, perhaps make a basic base class that handles the .__attrs_post_init__() then leave all the other classes clean (at least those that only need parent passed up).

@attr.s(auto_attrib=True)
class AttrsQObject:
    _parent: QtWidgets.QWidget = None

    def __attrs_post_init__(self):
        super().__init__(parent=self._parent)

Note the 'private' ._parent to avoid shadowing the QObject.parent() method. Yeah, that issue pre-exists this change, just noticed it. Ah, inheritance...

Copy link
Collaborator Author

@gordon-epc gordon-epc Mar 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I held off changing the wildcard declaration + initialization to a one liner. I did do the _parent. Again that's a good find.

What I found is that no matter what, there is no way to avoid adding the __attrs_post_init__ method. The inherited class init method has to be called and as far as I can tell the only way to do that is making a call through __attrs_post_init__.

Really, it doesn't make much sense to have the base class. It's not saving any lines of code, that's for sure. Let me know what you think.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having a general AttrsQObject, or whatever, is a bigger picture thing than this PR or these two classes. The sort of thing that is of at least uncertain value to backport, but ... might (?) be a good thing for future development. It would clearly avoid the .parent shadow issue in one place and allow perhaps many other classes to not have a .__attrs_post_init__() which would encourage them to do less in there which is one of the things I've accepted as good about attrs. No more, 'well I already have one so... do a bit more!'. :]

But yeah, big picture, various opinions, not really to be dealt with in this PR.


def lessThan(self, left, right):
Expand Down Expand Up @@ -750,17 +762,17 @@ def set_row_column(index, row=None, column=None):
return None


@attr.s
class DiffProxyModel(QtCore.QIdentityProxyModel):
parent = attr.ib(default=None)
columns = attr.ib(factory=set, converter=set)
_reference_column = attr.ib(default=None)
diff_highlights = attr.ib(factory=dict)
reference_highlights = attr.ib(factory=dict)
diff_role = attr.ib(default=QtCore.Qt.ItemDataRole.DisplayRole)

def __attrs_post_init__(self):
super().__init__(self.parent)
@attr.s(auto_attribs=True)
class HighlightDiffSortFilterProxyModel(SortFilterProxyModel):
columns: typing.Set[int] = attr.ib(factory=set, converter=set)
_reference_column: int = None
diff_highlights: typing.Dict[
QtCore.Qt.ItemDataRole, PyQt5.QtGui.QColor
] = attr.Factory(dict)
reference_highlights: typing.Dict[
QtCore.Qt.ItemDataRole, PyQt5.QtGui.QColor
] = attr.Factory(dict)
diff_role: QtCore.Qt.ItemDataRole = QtCore.Qt.ItemDataRole.DisplayRole

def data(self, index, role):
column = index.column()
Expand Down Expand Up @@ -857,23 +869,27 @@ def search_view(view, text, column):
return

model = view.model()

index = view.currentIndex()
models = []

while model is not None:
while True:
models.append(model)
search = getattr(model, "search", None)
if search is not None:
break

model = model.sourceModel()
else:
raise Exception("ack")
next_model = model.sourceModel()

if next_model is None:
raise Exception("ack")

index = model.mapToSource(index)
model = next_model

index = search(
text=text,
column=column,
search_from=view.currentIndex(),
search_from=index,
)

if index is not None:
Expand Down