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

PR: Add new "Plotting library" option, defaulting to Matplotlib (Variable Explorer) #20075

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open
8 changes: 3 additions & 5 deletions external-deps/spyder-kernels/spyder_kernels/console/start.py

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion spyder/config/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
from spyder.config.appearance import APPEARANCE
from spyder.plugins.editor.utils.findtasks import TASKS_PATTERN
from spyder.utils.introspection.module_completion import PREFERRED_MODULES
from spyder.plugins.variableexplorer.plotlib import DEFAULT_PLOTLIB


# =============================================================================
Expand Down Expand Up @@ -176,7 +177,8 @@
'truncate': True,
'minmax': False,
'show_callable_attributes': True,
'show_special_attributes': False
'show_special_attributes': False,
'plotlib': DEFAULT_PLOTLIB,
}),
('debugger',
{
Expand Down
1 change: 1 addition & 0 deletions spyder/plugins/ipythonconsole/utils/kernelspec.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ def env(self):
'SPY_GREEDY_O': self.get_conf('greedy_completer'),
'SPY_JEDI_O': self.get_conf('jedi_completer'),
'SPY_SYMPY_O': self.get_conf('symbolic_math'),
'SPY_VAREXP_PLOTLIB': self.get_conf('plotlib', section='variable_explorer'),
'SPY_TESTING': running_under_pytest() or get_safe_mode(),
'SPY_HIDE_CMD': self.get_conf('hide_cmd_windows'),
'SPY_PYTHONPATH': pypath
Expand Down
29 changes: 0 additions & 29 deletions spyder/plugins/plots/widgets/figurebrowser.py
Original file line number Diff line number Diff line change
Expand Up @@ -772,8 +772,6 @@ def add_thumbnail(self, fig, fmt):
parent=self, background_color=self.background_color)
thumbnail.canvas.load_figure(fig, fmt)
thumbnail.sig_canvas_clicked.connect(self.set_current_thumbnail)
thumbnail.sig_remove_figure_requested.connect(self.remove_thumbnail)
thumbnail.sig_save_figure_requested.connect(self.save_figure_as)
thumbnail.sig_context_menu_requested.connect(
lambda point: self.show_context_menu(point, thumbnail))
self._thumbnails.append(thumbnail)
Expand All @@ -796,8 +794,6 @@ def remove_all_thumbnails(self):
"""Remove all thumbnails."""
for thumbnail in self._thumbnails:
thumbnail.sig_canvas_clicked.disconnect()
thumbnail.sig_remove_figure_requested.disconnect()
thumbnail.sig_save_figure_requested.disconnect()
self.layout().removeWidget(thumbnail)
thumbnail.setParent(None)
thumbnail.hide()
Expand All @@ -815,8 +811,6 @@ def remove_thumbnail(self, thumbnail):
# Disconnect signals
try:
thumbnail.sig_canvas_clicked.disconnect()
thumbnail.sig_remove_figure_requested.disconnect()
thumbnail.sig_save_figure_requested.disconnect()
except TypeError:
pass

Expand Down Expand Up @@ -945,29 +939,6 @@ class FigureThumbnail(QWidget):
The clicked figure thumbnail.
"""

sig_remove_figure_requested = Signal(object)
"""
This signal is emitted to request the removal of a figure thumbnail.

Parameters
----------
figure_thumbnail: spyder.plugins.plots.widget.figurebrowser.FigureThumbnail
The figure thumbnail to remove.
"""

sig_save_figure_requested = Signal(object, str)
"""
This signal is emitted to request the saving of a figure thumbnail.

Parameters
----------
figure_thumbnail: spyder.plugins.plots.widget.figurebrowser.FigureThumbnail
The figure thumbnail to save.
format: str
The image format to use when saving the image. One of "image/png",
"image/jpeg" and "image/svg+xml".
"""

sig_context_menu_requested = Signal(QPoint)
"""
This signal is emitted to request a context menu.
Expand Down
29 changes: 23 additions & 6 deletions spyder/plugins/variableexplorer/confpage.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,12 @@
# Local imports
from spyder.config.base import _
from spyder.api.preferences import PluginConfigPage
from spyder.plugins.variableexplorer import plotlib

class VariableExplorerConfigPage(PluginConfigPage):

def setup_page(self):
# Filter Group
filter_group = QGroupBox(_("Filter"))
filter_data = [
('exclude_private', _("Exclude private references")),
Expand All @@ -27,20 +29,35 @@ def setup_page(self):
]
filter_boxes = [self.create_checkbox(text, option)
for option, text in filter_data]

display_group = QGroupBox(_("Display"))
display_data = [('minmax', _("Show arrays min/max"), '')]
display_boxes = [self.create_checkbox(text, option, tip=tip)
for option, text, tip in display_data]

filter_layout = QVBoxLayout()
for box in filter_boxes:
filter_layout.addWidget(box)
filter_group.setLayout(filter_layout)

# Display Group
display_group = QGroupBox(_("Display"))
display_data = [("minmax", _("Show arrays min/max"), "")]
display_boxes = [
self.create_checkbox(text, option, tip=tip)
for option, text, tip in display_data
]
display_layout = QVBoxLayout()
for box in display_boxes:
display_layout.addWidget(box)
if plotlib.AVAILABLE_PLOTLIBS:
plotlib_box = self.create_combobox(
_("Plotting library:") + " ",
zip(plotlib.AVAILABLE_PLOTLIBS, plotlib.AVAILABLE_PLOTLIBS),
"plotlib",
default=plotlib.DEFAULT_PLOTLIB,
tip=_(
"Default library used for data plotting of NumPy arrays "
"(curve, histogram, image).<br><br>Regarding the "
"<i>%varexp</i> magic command, this option will be "
"applied the next time a console is opened."
),
)
display_layout.addWidget(plotlib_box)
display_group.setLayout(display_layout)

vlayout = QVBoxLayout()
Expand Down
53 changes: 53 additions & 0 deletions spyder/plugins/variableexplorer/plotlib.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
# -*- coding: utf-8 -*-
#
# Copyright © Spyder Project Contributors
# Licensed under the terms of the MIT License
# (see spyder/__init__.py for details)

"""
Variable Explorer Plugin plotting library management.
"""

# Standard library imports
import importlib

# Local imports
from spyder.config.base import _


# Defining compatible plotting libraries
# (default library is the first available, see `get_default_plotlib`)
LIBRARIES = ("matplotlib", "guiqwt")
Copy link
Member

Choose a reason for hiding this comment

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

I think these names should be capitalized to look good in our Preferences. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Both library logos display their names in all lowercase, so I would suggest not bothering with capitalizing them, hence simplifying code (key == value, as for the combo box management).



def is_package_installed(modname):
"""Check if package is installed **without importing it**

Note: As Spyder won't start if matplotlib has been imported too early,
we do not use `utils.programs.is_module_installed` here because
it imports module to check if it's installed.
"""
return importlib.util.find_spec(modname) is not None


def get_available_plotlibs():
"""Return list of available plotting libraries"""
return [name for name in LIBRARIES if is_package_installed(name)]


def get_default_plotlib():
"""Return default plotting library"""
names = get_available_plotlibs()
if names is not None:
return names[0]
Copy link
Member

Choose a reason for hiding this comment

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

This function returns None if no library is available, which I think wouldn't look good in our Preferences. So, perhaps it should return Matplotlib?

Copy link
Contributor Author

@PierreRaybaut PierreRaybaut Nov 25, 2022

Choose a reason for hiding this comment

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

Actually that's not a problem because if there is no library available, the combo box won't be displayed in Preferences: combo box is added to Preferences page only if plotlib.AVAILABLE_PLOTLIBS is not empty. But I realize that it's not a good idea because even if plotlib.AVAILABLE_PLOTLIBS is empty, that does not mean that the IPython kernel won't have the library installed. So I'm rolling back on this.

And there is another problem: I've just did some tests and apparently Spyder Preferences do not handle well dynamic choices in combo boxes: if the choices are not the same as the previous run, the displayed choice in Preferences page will be changed but won't be considered as a new value when validating the dialog box. In other words, in that last case and if the list of choices is reduced to a single choice, it won't be possible to select the one and only choice available by validating the Preferences page...

So, I'm rolling back and proposing a static list for plotting libraries in the Preferences page. This should have some impact on other modules. I'll see about that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See 7adeece



def get_requirement_error_message():
"""Return html error message when no library is available"""
txt = ", ".join(["<b>%s</b>" % name for name in LIBRARIES])
return _("Please install a compatible plotting library (%s).") % txt


AVAILABLE_PLOTLIBS = get_available_plotlibs()
DEFAULT_PLOTLIB = get_default_plotlib()
REQ_ERROR_MSG = get_requirement_error_message()
18 changes: 0 additions & 18 deletions spyder/pyplot.py

This file was deleted.

100 changes: 50 additions & 50 deletions spyder/widgets/collectionseditor.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
from spyder.plugins.variableexplorer.widgets.importwizard import ImportWizard
from spyder.widgets.helperwidgets import CustomSortFilterProxy
from spyder.plugins.variableexplorer.widgets.basedialog import BaseDialog
from spyder.plugins.variableexplorer.plotlib import REQ_ERROR_MSG
from spyder.utils.palette import SpyderPalette
from spyder.utils.stylesheet import PANES_TOOLBAR_STYLESHEET

Expand Down Expand Up @@ -1132,57 +1133,41 @@ def view_item(self):
index = index.child(index.row(), 3)
self.delegate.createEditor(self, None, index, object_explorer=True)

def __prepare_plot(self):
try:
import guiqwt.pyplot #analysis:ignore
return True
except:
try:
if 'matplotlib' not in sys.modules:
import matplotlib # noqa
return True
except Exception:
QMessageBox.warning(self, _("Import error"),
_("Please install <b>matplotlib</b>"
" or <b>guiqwt</b>."))

def plot_item(self, funcname):
"""Plot item"""
index = self.currentIndex()
if self.__prepare_plot():
if self.proxy_model:
key = self.source_model.get_key(
self.proxy_model.mapToSource(index))
else:
key = self.source_model.get_key(index)
try:
self.plot(key, funcname)
except (ValueError, TypeError) as error:
QMessageBox.critical(self, _( "Plot"),
_("<b>Unable to plot data.</b>"
"<br><br>Error message:<br>%s"
) % str(error))
if self.proxy_model:
key = self.source_model.get_key(
self.proxy_model.mapToSource(index))
else:
key = self.source_model.get_key(index)
try:
self.plot(key, funcname)
except (ValueError, TypeError) as error:
QMessageBox.critical(self, _( "Plot"),
_("<b>Unable to plot data.</b>"
"<br><br>Error message:<br>%s"
) % str(error))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
QMessageBox.critical(self, _( "Plot"),
_("<b>Unable to plot data.</b>"
"<br><br>Error message:<br>%s"
) % str(error))
QMessageBox.critical(
self,
_( "Plot"),
_("<b>Unable to plot data.</b><br><br>"
"The error message was:<br>%s") % str(error)
)

Improve formatting and message a bit.

Copy link
Contributor Author

@PierreRaybaut PierreRaybaut Nov 25, 2022

Choose a reason for hiding this comment

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

Regarding code formatting, you really should consider using Black and force everyone contributing to do so. At work, all my teams are using Black together with isort with proper configuration (I gradually imposed this on all projects since 2018). CodraFT is an example of project following standard guidelines in terms of code quality.

Using (and forcing all contributors to use) Black saves time, increases code quality and helps focusing on real code changes (instead of being annoyed with code appearance).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I push this further? (by integrating Black into Spyder for example?)


@Slot()
def imshow_item(self):
"""Imshow item"""
index = self.currentIndex()
if self.__prepare_plot():
if self.proxy_model:
key = self.source_model.get_key(
self.proxy_model.mapToSource(index))
if self.proxy_model:
key = self.source_model.get_key(
self.proxy_model.mapToSource(index))
else:
key = self.source_model.get_key(index)
try:
if self.is_image(key):
self.show_image(key)
else:
key = self.source_model.get_key(index)
try:
if self.is_image(key):
self.show_image(key)
else:
self.imshow(key)
except (ValueError, TypeError) as error:
QMessageBox.critical(self, _( "Plot"),
_("<b>Unable to show image.</b>"
"<br><br>Error message:<br>%s"
) % str(error))
self.imshow(key)
except (ValueError, TypeError) as error:
QMessageBox.critical(self, _( "Plot"),
_("<b>Unable to show image.</b>"
"<br><br>Error message:<br>%s"
) % str(error))

@Slot()
def save_array(self):
Expand Down Expand Up @@ -1383,21 +1368,36 @@ def oedit(self, key):
oedit)
oedit(data[key])

def __get_pyplot(self):
"""Return default plotting library `pyplot` package if available.
(default plotting library is an option defined in Variable Explorer
plugin scope, e.g. "maplotlib" or "guiqwt")"""
libname = self.get_conf('plotlib', section='variable_explorer')
try:
return __import__(libname + '.pyplot',
globals(), locals(), [], 0).pyplot
except ImportError:
# This should not happen, unless plotting library has been
# uninstalled after option has been set in Variable Explorer
QMessageBox.critical(self, _("Import error"), REQ_ERROR_MSG)

def plot(self, key, funcname):
"""Plot item"""
data = self.source_model.get_data()
import spyder.pyplot as plt
plt.figure()
getattr(plt, funcname)(data[key])
plt.show()
plt = self.__get_pyplot()
if plt is not None:
plt.figure()
getattr(plt, funcname)(data[key])
plt.show()

def imshow(self, key):
"""Show item's image"""
data = self.source_model.get_data()
import spyder.pyplot as plt
plt.figure()
plt.imshow(data[key])
plt.show()
plt = self.__get_pyplot()
if plt is not None:
plt.figure()
plt.imshow(data[key])
plt.show()

def show_image(self, key):
"""Show image (item is a PIL image)"""
Expand Down