Apply suggestions from code review

* Fix style of refresh button in object explorer
* Mock correct function in test
* Many style fixes

Co-authored-by: Carlos Cordoba <[email protected]>

spyder/plugins/variableexplorer/widgets/arrayeditor.py

@@ -8,14 +8,13 @@
 NumPy Array Editor Dialog based on Qt
 """
 
-from __future__ import annotations
-
 # pylint: disable=C0103
 # pylint: disable=R0903
 # pylint: disable=R0911
 # pylint: disable=R0201
 
 # Standard library imports
+from __future__ import annotations
 import io
 from typing import Callable, Optional, TYPE_CHECKING
 
@@ -656,8 +655,11 @@ class ArrayEditor(BaseDialog, SpyderWidgetMixin):
 
     CONF_SECTION = 'variable_explorer'
 
-    def __init__(self, parent: Optional[QWidget] = None,
-                 data_function: Optional[Callable[[], ArrayLike]] = None):
+    def __init__(
+        self,
+        parent: Optional[QWidget] = None,
+        data_function: Optional[Callable[[], ArrayLike]] = None
+    ):
         """
         Constructor.
 
@@ -692,15 +694,16 @@ def __init__(self, parent: Optional[QWidget] = None,
 
     def setup_and_check(self, data, title='', readonly=False):
         """
-        Setup ArrayEditor:
-        return False if data is not supported, True otherwise
+        Setup the editor.
+
+        It returns False if data is not supported, True otherwise.
         """
         self.setup_ui(title, readonly)
         return self.set_data_and_check(data, readonly)
 
     def setup_ui(self, title='', readonly=False):
         """
-        Create user interface
+        Create the user interface.
 
         This creates the necessary widgets and layouts that make up the user
         interface of the array editor. Some elements need to be hidden
@@ -716,8 +719,8 @@ def setup_ui(self, title='', readonly=False):
 
         def do_nothing():
             # .create_action() needs a toggled= parameter, but we can only
-            # set it later in .set_data_and_check(), so we use this function
-            # as a placeholder here.
+            # set it later in the set_data_and_check method, so we use this
+            # function as a placeholder here.
             pass
 
         self.copy_action = self.create_action(
@@ -778,7 +781,8 @@ def do_nothing():
         # ---- Widgets in bottom left for special arrays
         #
         # These are normally hidden. When editing masked, record or 3d arrays,
-        # the relevant elements are made visible in `.set_data_and_check()`.
+        # the relevant elements are made visible in the set_data_and_check
+        # method.
 
         self.btn_layout = QHBoxLayout()
 
@@ -803,15 +807,17 @@ def do_nothing():
         self.btn_layout.addWidget(self.slicing_label)
 
         self.masked_label = QLabel(
-            _('<u>Warning</u>: Changes are applied separately'))
+            _('<u>Warning</u>: Changes are applied separately')
+        )
         self.masked_label.setToolTip(
             _("For performance reasons, changes applied to masked arrays won't"
-              "be reflected in array's data (and vice-versa)."))
+              "be reflected in array's data (and vice-versa).")
+          )
         self.btn_layout.addWidget(self.masked_label)
 
         self.btn_layout.addStretch()
 
-        # ---- Push buttons on bottom right
+        # ---- Push buttons on the bottom right
 
         self.btn_save_and_close = QPushButton(_('Save and Close'))
         self.btn_save_and_close.setDisabled(True)
@@ -1024,9 +1030,11 @@ def current_widget_changed(self, index):
         self.arraywidget = self.stack.widget(index)
         if self.arraywidget:
             self.arraywidget.model.dataChanged.connect(
-                self.save_and_close_enable)
+                self.save_and_close_enable
+            )
             self.toggle_bgcolor_action.setChecked(
-                self.arraywidget.model.bgcolor_enabled)
+                self.arraywidget.model.bgcolor_enabled
+            )
 
     def change_active_widget(self, index):
         """
@@ -1100,7 +1108,8 @@ def refresh(self) -> None:
 
         if not self.set_data_and_check(data):
             self.error(
-                _('The new value cannot be displayed in the array editor.'))
+                _('The new value cannot be displayed in the array editor.')
+            )
 
     def ask_for_refresh_confirmation(self) -> bool:
         """
@@ -1113,7 +1122,10 @@ def ask_for_refresh_confirmation(self) -> bool:
         message = _('Refreshing the editor will overwrite the changes that '
                     'you made. Do you want to proceed?')
         result = QMessageBox.question(
-            self, _('Refresh array editor?'), message)
+            self,
+            _('Refresh array editor?'),
+            message
+        )
         return result == QMessageBox.Yes
 
     @Slot()

spyder/plugins/variableexplorer/widgets/collectionsdelegate.py

@@ -45,8 +45,12 @@ class CollectionsDelegate(QItemDelegate, SpyderFontsMixin):
     sig_editor_creation_started = Signal()
     sig_editor_shown = Signal()
 
-    def __init__(self, parent=None, namespacebrowser=None,
-                 data_function: Optional[Callable[[], Any]] = None):
+    def __init__(
+        self,
+        parent=None,
+        namespacebrowser=None,
+        data_function: Optional[Callable[[], Any]] = None
+    ):
         QItemDelegate.__init__(self, parent)
         self.namespacebrowser = namespacebrowser
         self.data_function = data_function
@@ -60,8 +64,10 @@ def set_value(self, index, value):
         if index.isValid():
             index.model().set_value(index, value)
 
-    def make_data_function(self, index: QModelIndex
-                           ) -> Optional[Callable[[], Any]]:
+    def make_data_function(
+        self,
+        index: QModelIndex
+    ) -> Optional[Callable[[], Any]]:
         """
         Construct function which returns current value of data.
 
@@ -92,6 +98,7 @@ def datafun():
             data = self.data_function()
             if isinstance(data, (tuple, list, dict, set)):
                 return data[key]
+
             try:
                 return getattr(data, key)
             except (NotImplementedError, AttributeError,
@@ -207,8 +214,10 @@ def createEditor(self, parent, option, index, object_explorer=False):
         elif isinstance(value, (list, set, tuple, dict)) and not object_explorer:
             from spyder.widgets.collectionseditor import CollectionsEditor
             editor = CollectionsEditor(
-                parent=parent, namespacebrowser=self.namespacebrowser,
-                data_function=self.make_data_function(index))
+                parent=parent,
+                namespacebrowser=self.namespacebrowser,
+                data_function=self.make_data_function(index)
+            )
             editor.setup(value, key, icon=self.parent().windowIcon(),
                          readonly=readonly)
             self.create_dialog(editor, dict(model=index.model(), editor=editor,
@@ -220,7 +229,9 @@ def createEditor(self, parent, option, index, object_explorer=False):
             # We need to leave this import here for tests to pass.
             from .arrayeditor import ArrayEditor
             editor = ArrayEditor(
-                parent=parent, data_function=self.make_data_function(index))
+                parent=parent,
+                data_function=self.make_data_function(index)
+            )
             if not editor.setup_and_check(value, title=key, readonly=readonly):
                 self.sig_editor_shown.emit()
                 return
@@ -252,7 +263,9 @@ def createEditor(self, parent, option, index, object_explorer=False):
             # We need to leave this import here for tests to pass.
             from .dataframeeditor import DataFrameEditor
             editor = DataFrameEditor(
-                parent=parent, data_function=self.make_data_function(index))
+                parent=parent,
+                data_function=self.make_data_function(index)
+            )
             if not editor.setup_and_check(value, title=key):
                 self.sig_editor_shown.emit()
                 return
@@ -466,7 +479,8 @@ class ToggleColumnDelegate(CollectionsDelegate):
     def __init__(self, parent=None, namespacebrowser=None,
                  data_function: Optional[Callable[[], Any]] = None):
         CollectionsDelegate.__init__(
-            self, parent, namespacebrowser, data_function)
+            self, parent, namespacebrowser, data_function
+        )
         self.current_index = None
         self.old_obj = None
 
@@ -486,8 +500,10 @@ def set_value(self, index, value):
         if index.isValid():
             index.model().set_value(index, value)
 
-    def make_data_function(self, index: QModelIndex
-                           ) -> Optional[Callable[[], Any]]:
+    def make_data_function(
+        self,
+        index: QModelIndex
+    ) -> Optional[Callable[[], Any]]:
         """
         Construct function which returns current value of data.
 
@@ -565,8 +581,10 @@ def createEditor(self, parent, option, index):
         if isinstance(value, (list, set, tuple, dict)):
             from spyder.widgets.collectionseditor import CollectionsEditor
             editor = CollectionsEditor(
-                parent=parent, namespacebrowser=self.namespacebrowser,
-                data_function=self.make_data_function(index))
+                parent=parent,
+                namespacebrowser=self.namespacebrowser,
+                data_function=self.make_data_function(index)
+            )
             editor.setup(value, key, icon=self.parent().windowIcon(),
                          readonly=readonly)
             self.create_dialog(editor, dict(model=index.model(), editor=editor,
@@ -576,7 +594,9 @@ def createEditor(self, parent, option, index):
         elif (isinstance(value, (np.ndarray, np.ma.MaskedArray)) and
                 np.ndarray is not FakeObject):
             editor = ArrayEditor(
-                parent=parent, data_function=self.make_data_function(index))
+                parent=parent,
+                data_function=self.make_data_function(index)
+            )
             if not editor.setup_and_check(value, title=key, readonly=readonly):
                 return
             self.create_dialog(editor, dict(model=index.model(), editor=editor,
@@ -598,7 +618,9 @@ def createEditor(self, parent, option, index):
         elif (isinstance(value, (pd.DataFrame, pd.Index, pd.Series))
                 and pd.DataFrame is not FakeObject):
             editor = DataFrameEditor(
-                parent=parent, data_function=self.make_data_function(index))
+                parent=parent,
+                data_function=self.make_data_function(index)
+            )
             if not editor.setup_and_check(value, title=key):
                 return
             self.create_dialog(editor, dict(model=index.model(), editor=editor,

spyder/plugins/variableexplorer/widgets/dataframeeditor.py

@@ -788,7 +788,8 @@ def setup_menu(self):
             self, _('Refresh'),
             icon=ima.icon('refresh'),
             tip=_('Refresh editor with current value of variable in console'),
-            triggered=lambda: self.sig_refresh_requested.emit())
+            triggered=lambda: self.sig_refresh_requested.emit()
+        )
         self.refresh_action.setEnabled(self.data_function is not None)
 
         self.convert_to_action = create_action(self, _('Convert to'))
@@ -1612,8 +1613,11 @@ class DataFrameEditor(BaseDialog, SpyderConfigurationAccessor):
     """
     CONF_SECTION = 'variable_explorer'
 
-    def __init__(self, parent: QWidget = None,
-                 data_function: Optional[Callable[[], Any]] = None):
+    def __init__(
+        self,
+        parent: QWidget = None,
+        data_function: Optional[Callable[[], Any]] = None
+    ):
         super().__init__(parent)
         self.data_function = data_function
 
@@ -1630,20 +1634,22 @@ def __init__(self, parent: QWidget = None,
 
     def setup_and_check(self, data, title='') -> bool:
         """
-        Setup DataFrameEditor:
-        return False if data is not supported, True otherwise.
-        Supported types for data are DataFrame, Series and Index.
+        Setup editor.
+        
+        It returns False if data is not supported, True otherwise. Supported
+        types for data are DataFrame, Series and Index.
         """
         if title:
             title = to_text_string(title) + " - %s" % data.__class__.__name__
         else:
             title = _("%s editor") % data.__class__.__name__
+
         self.setup_ui(title)
         return self.set_data_and_check(data)
 
     def setup_ui(self, title: str) -> None:
         """
-        Create user interface
+        Create user interface.
         """
         self.layout = QVBoxLayout()
         self.layout.setSpacing(0)
@@ -1718,9 +1724,9 @@ def setup_ui(self, title: str) -> None:
 
     def set_data_and_check(self, data) -> bool:
         """
-        Checks whether data is suitable and display it in the editor
+        Checks whether data is suitable and display it in the editor.
 
-        This function returns False if data is not supported.
+        This method returns False if data is not supported.
         """
         if not isinstance(data, (pd.DataFrame, pd.Series, pd.Index)):
             return False
@@ -2111,7 +2117,8 @@ def refresh_editor(self) -> None:
         if not self.set_data_and_check(data):
             self.error(
                 _('The new value cannot be displayed in the dataframe '
-                  'editor.'))
+                  'editor.')
+            )
 
     def ask_for_refresh_confirmation(self) -> bool:
         """
@@ -2124,7 +2131,10 @@ def ask_for_refresh_confirmation(self) -> bool:
         message = _('Refreshing the editor will overwrite the changes that '
                     'you made. Do you want to proceed?')
         result = QMessageBox.question(
-            self, _('Refresh dataframe editor?'), message)
+            self,
+            _('Refresh dataframe editor?'),
+            message
+        )
         return result == QMessageBox.Yes
 
     def error(self, message):

spyder/plugins/variableexplorer/widgets/objectexplorer/objectexplorer.py

@@ -143,7 +143,9 @@ def set_value(self, obj):
         # Tree widget
         old_obj_tree = self.obj_tree
         self.obj_tree = ToggleColumnTreeView(
-            self.namespacebrowser, self.data_function)
+            self.namespacebrowser,
+            self.data_function
+        )
         self.obj_tree.setAlternatingRowColors(True)
         self.obj_tree.setModel(self._proxy_tree_model)
         self.obj_tree.setSelectionBehavior(QAbstractItemView.SelectRows)
@@ -257,8 +259,10 @@ def _setup_menu(self, show_callable_attributes=False,
         self.refresh_button = create_toolbutton(
             self, icon=ima.icon('refresh'),
             tip=_('Refresh editor with current value of variable in console'),
-            triggered=self.refresh_editor)
+            triggered=self.refresh_editor
+        )
         self.refresh_button.setEnabled(self.data_function is not None)
+        self.refresh_button.setStyleSheet(str(PANES_TOOLBAR_STYLESHEET))
         self.tools_layout.addSpacing(5)
         self.tools_layout.addWidget(self.refresh_button)
 
@@ -419,8 +423,11 @@ def refresh_editor(self) -> None:
         try:
             data = self.data_function()
         except (IndexError, KeyError):
-            QMessageBox.critical(self, _('Collection editor'),
-                                 _('The variable no longer exists.'))
+            QMessageBox.critical(
+                self,
+                _('Object explorer'),
+                _('The variable no longer exists.')
+            )
             self.reject()
             return