From 127b59274c9bc8160f7af1e34aa4a8cd12fe7840 Mon Sep 17 00:00:00 2001 From: Eivind Jahren Date: Fri, 14 Jun 2024 09:51:55 +0200 Subject: [PATCH 1/7] Refactor to taking str in SuggestorMessage --- src/ert/gui/suggestor/_suggestor_message.py | 40 +++++++++++++-------- src/ert/gui/suggestor/suggestor.py | 18 ++++++---- 2 files changed, 37 insertions(+), 21 deletions(-) diff --git a/src/ert/gui/suggestor/_suggestor_message.py b/src/ert/gui/suggestor/_suggestor_message.py index 5037d30f22b..a6be5283d8f 100644 --- a/src/ert/gui/suggestor/_suggestor_message.py +++ b/src/ert/gui/suggestor/_suggestor_message.py @@ -1,7 +1,5 @@ from __future__ import annotations -from typing import TYPE_CHECKING - from PyQt5 import QtSvg from PyQt5.QtCore import Qt from PyQt5.QtGui import QColor @@ -23,9 +21,6 @@ YELLOW_TEXT, ) -if TYPE_CHECKING: - from ert.config import ErrorInfo, WarningInfo - def _svg_icon(image_name: str) -> QtSvg.QSvgWidget: widget = QtSvg.QSvgWidget(f"img:{image_name}.svg") @@ -40,7 +35,8 @@ def __init__( text_color: str, bg_color: str, icon: QWidget, - info: ErrorInfo, + message: str, + location: str, ) -> None: super().__init__() self.setAttribute(Qt.WidgetAttribute.WA_StyledBackground) @@ -59,15 +55,15 @@ def __init__( self.setContentsMargins(0, 0, 0, 0) self.icon = icon - info.message = info.message.replace("<", "<").replace(">", ">") + message = message.replace("<", "<").replace(">", ">") self.lbl = QLabel( '
' + f'' + header + "" - + info.message + + message + "

" - + info.location() + + location + "

" + "
" ) @@ -82,15 +78,29 @@ def __init__( self.setLayout(self.hbox) @classmethod - def error_msg(cls, info: ErrorInfo) -> Self: - return cls("Error: ", RED_TEXT, RED_BACKGROUND, _svg_icon("error"), info) + def error_msg(cls, message: str, location: str) -> Self: + return cls( + "Error: ", RED_TEXT, RED_BACKGROUND, _svg_icon("error"), message, location + ) @classmethod - def warning_msg(cls, info: WarningInfo) -> Self: + def warning_msg(cls, message: str, location: str) -> Self: return cls( - "Warning: ", YELLOW_TEXT, YELLOW_BACKGROUND, _svg_icon("warning"), info + "Warning: ", + YELLOW_TEXT, + YELLOW_BACKGROUND, + _svg_icon("warning"), + message, + location, ) @classmethod - def deprecation_msg(cls, info: WarningInfo) -> Self: - return cls("Deprecation: ", BLUE_TEXT, BLUE_BACKGROUND, _svg_icon("bell"), info) + def deprecation_msg(cls, message: str, location: str) -> Self: + return cls( + "Deprecation: ", + BLUE_TEXT, + BLUE_BACKGROUND, + _svg_icon("bell"), + message, + location, + ) diff --git a/src/ert/gui/suggestor/suggestor.py b/src/ert/gui/suggestor/suggestor.py index df62218bcbb..6ddc2446ecf 100644 --- a/src/ert/gui/suggestor/suggestor.py +++ b/src/ert/gui/suggestor/suggestor.py @@ -241,20 +241,26 @@ def _messages( column = 0 row = 0 num = 0 - for msg in errors: - suggest_layout.addWidget(SuggestorMessage.error_msg(msg), row, column) + for err in errors: + suggest_layout.addWidget( + SuggestorMessage.error_msg(err.message, err.location()), row, column + ) if column: row += 1 column = (column + 1) % NUM_COLUMNS num += 1 - for msg in warnings: - suggest_layout.addWidget(SuggestorMessage.warning_msg(msg), row, column) + for w in warnings: + suggest_layout.addWidget( + SuggestorMessage.warning_msg(w.message, w.location()), row, column + ) if column: row += 1 column = (column + 1) % NUM_COLUMNS num += 1 - for msg in deprecations: - suggest_layout.addWidget(SuggestorMessage.deprecation_msg(msg), row, column) + for d in deprecations: + suggest_layout.addWidget( + SuggestorMessage.deprecation_msg(d.message, d.location()), row, column + ) if column: row += 1 column = (column + 1) % NUM_COLUMNS From a5fb67e0c105b6595037f7b62e520d3efb8d7633 Mon Sep 17 00:00:00 2001 From: Eivind Jahren Date: Fri, 14 Jun 2024 09:56:54 +0200 Subject: [PATCH 2/7] Refactor to take more than one location --- src/ert/gui/suggestor/_suggestor_message.py | 23 ++++++++++++++------- src/ert/gui/suggestor/suggestor.py | 6 +++--- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/ert/gui/suggestor/_suggestor_message.py b/src/ert/gui/suggestor/_suggestor_message.py index a6be5283d8f..3dde21fac1b 100644 --- a/src/ert/gui/suggestor/_suggestor_message.py +++ b/src/ert/gui/suggestor/_suggestor_message.py @@ -36,7 +36,7 @@ def __init__( bg_color: str, icon: QWidget, message: str, - location: str, + locations: list[str], ) -> None: super().__init__() self.setAttribute(Qt.WidgetAttribute.WA_StyledBackground) @@ -56,6 +56,13 @@ def __init__( self.icon = icon message = message.replace("<", "<").replace(">", ">") + + location_paragraph = "" + if locations: + location_paragraph = locations[0] + if len(locations) > 1: + location_paragraph += f" and {len(locations) - 1} more" + self.lbl = QLabel( '
' + f'' @@ -63,7 +70,7 @@ def __init__( + "" + message + "

" - + location + + location_paragraph + "

" + "
" ) @@ -78,29 +85,29 @@ def __init__( self.setLayout(self.hbox) @classmethod - def error_msg(cls, message: str, location: str) -> Self: + def error_msg(cls, message: str, locations: list[str]) -> Self: return cls( - "Error: ", RED_TEXT, RED_BACKGROUND, _svg_icon("error"), message, location + "Error: ", RED_TEXT, RED_BACKGROUND, _svg_icon("error"), message, locations ) @classmethod - def warning_msg(cls, message: str, location: str) -> Self: + def warning_msg(cls, message: str, locations: list[str]) -> Self: return cls( "Warning: ", YELLOW_TEXT, YELLOW_BACKGROUND, _svg_icon("warning"), message, - location, + locations, ) @classmethod - def deprecation_msg(cls, message: str, location: str) -> Self: + def deprecation_msg(cls, message: str, locations: list[str]) -> Self: return cls( "Deprecation: ", BLUE_TEXT, BLUE_BACKGROUND, _svg_icon("bell"), message, - location, + locations, ) diff --git a/src/ert/gui/suggestor/suggestor.py b/src/ert/gui/suggestor/suggestor.py index 6ddc2446ecf..1d3c5441209 100644 --- a/src/ert/gui/suggestor/suggestor.py +++ b/src/ert/gui/suggestor/suggestor.py @@ -243,7 +243,7 @@ def _messages( num = 0 for err in errors: suggest_layout.addWidget( - SuggestorMessage.error_msg(err.message, err.location()), row, column + SuggestorMessage.error_msg(err.message, [err.location()]), row, column ) if column: row += 1 @@ -251,7 +251,7 @@ def _messages( num += 1 for w in warnings: suggest_layout.addWidget( - SuggestorMessage.warning_msg(w.message, w.location()), row, column + SuggestorMessage.warning_msg(w.message, [w.location()]), row, column ) if column: row += 1 @@ -259,7 +259,7 @@ def _messages( num += 1 for d in deprecations: suggest_layout.addWidget( - SuggestorMessage.deprecation_msg(d.message, d.location()), row, column + SuggestorMessage.deprecation_msg(d.message, [d.location()]), row, column ) if column: row += 1 From b1ae51e7b5e4c4334ddb6a13676650957c052fc4 Mon Sep 17 00:00:00 2001 From: Eivind Jahren Date: Fri, 14 Jun 2024 10:44:23 +0200 Subject: [PATCH 3/7] Combine warnings & errors with same message --- src/ert/gui/suggestor/suggestor.py | 25 ++++++++++++++++--------- tests/unit_tests/gui/test_suggestor.py | 23 +++++++++++++++++++++++ 2 files changed, 39 insertions(+), 9 deletions(-) create mode 100644 tests/unit_tests/gui/test_suggestor.py diff --git a/src/ert/gui/suggestor/suggestor.py b/src/ert/gui/suggestor/suggestor.py index 1d3c5441209..5f8f397f3dc 100644 --- a/src/ert/gui/suggestor/suggestor.py +++ b/src/ert/gui/suggestor/suggestor.py @@ -3,7 +3,8 @@ import functools import logging import webbrowser -from typing import TYPE_CHECKING, Callable, Dict, List, Optional +from collections import defaultdict +from typing import TYPE_CHECKING, Callable, Dict, List, Optional, Sequence from PyQt5.QtGui import QCursor from qtpy.QtCore import Qt @@ -33,6 +34,13 @@ def _clicked_help_button(menu_label: str, link: str) -> None: webbrowser.open(link) +def _combine_messages(infos: Sequence[ErrorInfo]) -> list[tuple[str, list[str]]]: + combined = defaultdict(list) + for info in infos: + combined[info.message].append(info.location()) + return list(combined.items()) + + LIGHT_GREY = "#f7f7f7" MEDIUM_GREY = "#eaeaea" HEAVY_GREY = "#dcdcdc" @@ -232,6 +240,7 @@ def _messages( NUM_COLUMNS = 2 suggest_msgs = QWidget(parent=self) + suggest_msgs.setObjectName("suggestor_messages") suggest_msgs.setContentsMargins(0, 0, 16, 0) suggest_layout = QGridLayout() suggest_layout.setContentsMargins(0, 0, 0, 0) @@ -241,25 +250,23 @@ def _messages( column = 0 row = 0 num = 0 - for err in errors: - suggest_layout.addWidget( - SuggestorMessage.error_msg(err.message, [err.location()]), row, column - ) + for combined in _combine_messages(errors): + suggest_layout.addWidget(SuggestorMessage.error_msg(*combined), row, column) if column: row += 1 column = (column + 1) % NUM_COLUMNS num += 1 - for w in warnings: + for combined in _combine_messages(warnings): suggest_layout.addWidget( - SuggestorMessage.warning_msg(w.message, [w.location()]), row, column + SuggestorMessage.warning_msg(*combined), row, column ) if column: row += 1 column = (column + 1) % NUM_COLUMNS num += 1 - for d in deprecations: + for combined in _combine_messages(deprecations): suggest_layout.addWidget( - SuggestorMessage.deprecation_msg(d.message, [d.location()]), row, column + SuggestorMessage.deprecation_msg(*combined), row, column ) if column: row += 1 diff --git a/tests/unit_tests/gui/test_suggestor.py b/tests/unit_tests/gui/test_suggestor.py new file mode 100644 index 00000000000..a04366e07cc --- /dev/null +++ b/tests/unit_tests/gui/test_suggestor.py @@ -0,0 +1,23 @@ +import pytest +from qtpy.QtWidgets import QWidget + +from ert.config import ErrorInfo +from ert.gui.suggestor import Suggestor + + +@pytest.mark.parametrize( + "errors, expected_num", + [ + ([ErrorInfo("msg_1")], 1), + ([ErrorInfo("msg_1"), ErrorInfo("msg_2")], 2), + ([ErrorInfo("msg_1"), ErrorInfo("msg_1"), ErrorInfo("msg_2")], 2), + ([ErrorInfo("msg_1"), ErrorInfo("msg_2"), ErrorInfo("msg_3")], 3), + ], +) +def test_suggestor_combines_errors_with_the_same_message(qtbot, errors, expected_num): + suggestor = Suggestor(errors, [], [], lambda: None) + msgs = suggestor.findChild(QWidget, name="suggestor_messages") + assert msgs is not None + msg_layout = msgs.layout() + assert msg_layout is not None + assert msg_layout.count() == expected_num From ffe2b7fb498668339dd0455c7ee60e8473212e59 Mon Sep 17 00:00:00 2001 From: Eivind Jahren Date: Fri, 14 Jun 2024 12:04:09 +0200 Subject: [PATCH 4/7] Implement expand/collapse link --- src/ert/gui/suggestor/_suggestor_message.py | 104 ++++++++++++++++---- 1 file changed, 83 insertions(+), 21 deletions(-) diff --git a/src/ert/gui/suggestor/_suggestor_message.py b/src/ert/gui/suggestor/_suggestor_message.py index 3dde21fac1b..fdafa873d46 100644 --- a/src/ert/gui/suggestor/_suggestor_message.py +++ b/src/ert/gui/suggestor/_suggestor_message.py @@ -8,9 +8,10 @@ QHBoxLayout, QLabel, QSizePolicy, + QVBoxLayout, QWidget, ) -from typing_extensions import Self +from typing_extensions import Any, Self from ._colors import ( BLUE_BACKGROUND, @@ -54,35 +55,96 @@ def __init__( self.setGraphicsEffect(shadowEffect) self.setContentsMargins(0, 0, 0, 0) - self.icon = icon - message = message.replace("<", "<").replace(">", ">") + self._icon = icon + self._message = message.replace("<", "<").replace(">", ">") + self._locations = locations + self._header = header + self._text_color = text_color + self._hbox = QHBoxLayout() + self._hbox.setContentsMargins(16, 16, 16, 16) + self._hbox.addWidget(self._icon, alignment=Qt.AlignmentFlag.AlignTop) + self.setLayout(self._hbox) + + self.lbl = QLabel(self._collapsed_text()) + self.lbl.setOpenExternalLinks(False) + self.lbl.setSizePolicy(QSizePolicy.Expanding, QSizePolicy.Expanding) + self.lbl.setTextInteractionFlags(Qt.TextInteractionFlag.TextSelectableByMouse) + self.lbl.setWordWrap(True) + self._expanded = False + if len(self._locations) > 1: + self._expand_collapse_label = QLabel(self._expand_link()) + self._expand_collapse_label.setOpenExternalLinks(False) + self._expand_collapse_label.linkActivated.connect(self._toggle_expand) + + self._vbox = QWidget() + layout = QVBoxLayout() + self._vbox.setLayout(layout) + layout.addWidget(self.lbl) + layout.addWidget(self._expand_collapse_label) + self._hbox.addWidget(self._vbox, alignment=Qt.AlignmentFlag.AlignTop) + else: + self._expand_collapse_label = QLabel() + self._hbox.addWidget(self.lbl, alignment=Qt.AlignmentFlag.AlignTop) + + def _collapsed_text(self) -> str: location_paragraph = "" - if locations: - location_paragraph = locations[0] - if len(locations) > 1: - location_paragraph += f" and {len(locations) - 1} more" + if self._locations: + location_paragraph = self._locations[0] + location_paragraph = ( + "

" + + f' location: ' + + location_paragraph + + "

" + ) - self.lbl = QLabel( + return ( '
' - + f'' - + header + + f'' + + self._header + "" - + message - + "

" + + self._message + location_paragraph - + "

" + "
" ) - self.lbl.setSizePolicy(QSizePolicy.Expanding, QSizePolicy.Expanding) - self.lbl.setTextInteractionFlags(Qt.TextInteractionFlag.TextSelectableByMouse) - self.lbl.setWordWrap(True) - self.hbox = QHBoxLayout() - self.hbox.setContentsMargins(16, 16, 16, 16) - self.hbox.addWidget(self.icon, alignment=Qt.AlignmentFlag.AlignTop) - self.hbox.addWidget(self.lbl, alignment=Qt.AlignmentFlag.AlignTop) - self.setLayout(self.hbox) + def _toggle_expand(self, _link: Any) -> None: + if self._expanded: + self.lbl.setText(self._collapsed_text()) + self._expand_collapse_label.setText(self._expand_link()) + self._expanded = False + else: + self.lbl.setText(self._expanded_text()) + self._expand_collapse_label.setText(self._hide_link()) + self._expanded = True + + def _hide_link(self) -> str: + return " show less" + + def _expand_link(self) -> str: + return f" and {len(self._locations) - 1} more" + + def _expanded_text(self) -> str: + location_paragraphs = "" + first = True + for loc in self._locations: + if first: + location_paragraphs += ( + f'

location:{loc}

' + ) + first = False + else: + location_paragraphs += f"

{loc}

" + + return ( + '
' + + f'' + + self._header + + "" + + self._message + + location_paragraphs + + "
" + ) @classmethod def error_msg(cls, message: str, locations: list[str]) -> Self: From f6d3aa74bc74ad03ebaf72f9a00278d088114896 Mon Sep 17 00:00:00 2001 From: Eivind Jahren Date: Mon, 17 Jun 2024 09:02:45 +0200 Subject: [PATCH 5/7] Extract duplicated SuggestorMessage._text --- src/ert/gui/suggestor/_suggestor_message.py | 39 +++++++++------------ 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/src/ert/gui/suggestor/_suggestor_message.py b/src/ert/gui/suggestor/_suggestor_message.py index fdafa873d46..0ccb5c3b9e5 100644 --- a/src/ert/gui/suggestor/_suggestor_message.py +++ b/src/ert/gui/suggestor/_suggestor_message.py @@ -87,27 +87,6 @@ def __init__( self._expand_collapse_label = QLabel() self._hbox.addWidget(self.lbl, alignment=Qt.AlignmentFlag.AlignTop) - def _collapsed_text(self) -> str: - location_paragraph = "" - if self._locations: - location_paragraph = self._locations[0] - location_paragraph = ( - "

" - + f' location: ' - + location_paragraph - + "

" - ) - - return ( - '
' - + f'' - + self._header - + "" - + self._message - + location_paragraph - + "
" - ) - def _toggle_expand(self, _link: Any) -> None: if self._expanded: self.lbl.setText(self._collapsed_text()) @@ -124,6 +103,19 @@ def _hide_link(self) -> str: def _expand_link(self) -> str: return f" and {len(self._locations) - 1} more" + def _collapsed_text(self) -> str: + location_paragraph = "" + if self._locations: + location_paragraph = self._locations[0] + location_paragraph = ( + "

" + + f' location: ' + + location_paragraph + + "

" + ) + + return self._text(location_paragraph) + def _expanded_text(self) -> str: location_paragraphs = "" first = True @@ -136,13 +128,16 @@ def _expanded_text(self) -> str: else: location_paragraphs += f"

{loc}

" + return self._text(location_paragraphs) + + def _text(self, location: str) -> str: return ( '
' + f'' + self._header + "" + self._message - + location_paragraphs + + location + "
" ) From 9cc9234942de5e7d40c8dce52fadb6bfb501c99a Mon Sep 17 00:00:00 2001 From: Eivind Jahren Date: Mon, 17 Jun 2024 09:03:39 +0200 Subject: [PATCH 6/7] Simplify boolean toggle in _toggle_expand --- src/ert/gui/suggestor/_suggestor_message.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ert/gui/suggestor/_suggestor_message.py b/src/ert/gui/suggestor/_suggestor_message.py index 0ccb5c3b9e5..30647f96f6d 100644 --- a/src/ert/gui/suggestor/_suggestor_message.py +++ b/src/ert/gui/suggestor/_suggestor_message.py @@ -91,11 +91,10 @@ def _toggle_expand(self, _link: Any) -> None: if self._expanded: self.lbl.setText(self._collapsed_text()) self._expand_collapse_label.setText(self._expand_link()) - self._expanded = False else: self.lbl.setText(self._expanded_text()) self._expand_collapse_label.setText(self._hide_link()) - self._expanded = True + self._expanded = not self._expanded def _hide_link(self) -> str: return " show less" From b6dd0fba4b00be61826b25b3ee2b7b21536a8c4c Mon Sep 17 00:00:00 2001 From: Eivind Jahren Date: Mon, 17 Jun 2024 09:08:38 +0200 Subject: [PATCH 7/7] Extract duplicated SuggestorMessage._color_bold --- src/ert/gui/suggestor/_suggestor_message.py | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/ert/gui/suggestor/_suggestor_message.py b/src/ert/gui/suggestor/_suggestor_message.py index 30647f96f6d..126c58dab5d 100644 --- a/src/ert/gui/suggestor/_suggestor_message.py +++ b/src/ert/gui/suggestor/_suggestor_message.py @@ -107,10 +107,7 @@ def _collapsed_text(self) -> str: if self._locations: location_paragraph = self._locations[0] location_paragraph = ( - "

" - + f' location: ' - + location_paragraph - + "

" + "

" + self._color_bold("location: ") + location_paragraph + "

" ) return self._text(location_paragraph) @@ -120,9 +117,7 @@ def _expanded_text(self) -> str: first = True for loc in self._locations: if first: - location_paragraphs += ( - f'

location:{loc}

' - ) + location_paragraphs += f'

{self._color_bold("location:")}{loc}

' first = False else: location_paragraphs += f"

{loc}

" @@ -132,14 +127,15 @@ def _expanded_text(self) -> str: def _text(self, location: str) -> str: return ( '
' - + f'' - + self._header - + "" + + self._color_bold(self._header) + self._message + location + "
" ) + def _color_bold(self, text: str) -> str: + return f'{text}' + @classmethod def error_msg(cls, message: str, locations: list[str]) -> Self: return cls(