Repository navigation
Description
Inside my Jupyter Notebooks, I want to create several plots with matplotlib. Since the release of v0.1.5, my CI for my docs and unittests is failing now.
The error message goes like this:
File ~/work/conflowgen/conflowgen/conflowgen/analyses/inbound_and_outbound_vehicle_capacity_analysis_report.py:77, in InboundAndOutboundVehicleCapacityAnalysisReport.get_report_as_graph(self, **kwargs)
71 df = pd.DataFrame({
72 "inbound volume (in TEU)": inbound_capacities,
73 "outbound volume (in TEU)": outbound_actual_capacities,
74 "outbound maximum capacity": outbound_maximum_capacities
75 })
76 df.index = [str(i).replace("_", " ") for i in df.index]
---> 77 ax = df.plot.barh()
78 ax.set_xlabel("Capacity (in TEU)")
79 ax.set_title("Inbound and outbound vehicle capacity analysis")
File ~/.local/lib/python3.8/site-packages/pandas/plotting/_core.py:1217, in PlotAccessor.barh(self, x, y, **kwargs)
1133 @Appender(
1134 """
1135 See Also
(...)
1206 @Appender(_bar_or_line_doc)
1207 def barh(self, x=None, y=None, **kwargs):
1208 """
1209 Make a horizontal bar plot.
1210
(...)
1215 other axis represents a measured value.
1216 """
-> 1217 return self(kind="barh", x=x, y=y, **kwargs)
File ~/.local/lib/python3.8/site-packages/pandas/plotting/_core.py:972, in PlotAccessor.__call__(self, *args, **kwargs)
969 label_name = label_kw or data.columns
970 data.columns = label_name
--> 972 return plot_backend.plot(data, kind=kind, **kwargs)
File ~/.local/lib/python3.8/site-packages/pandas/plotting/_matplotlib/__init__.py:71, in plot(data, kind, **kwargs)
69 kwargs["ax"] = getattr(ax, "left_ax", ax)
70 plot_obj = PLOT_CLASSES[kind](data, **kwargs)
---> 71 plot_obj.generate()
72 plot_obj.draw()
73 return plot_obj.result
File ~/.local/lib/python3.8/site-packages/pandas/plotting/_matplotlib/core.py:328, in MPLPlot.generate(self)
326 self._args_adjust()
327 self._compute_plot_data()
--> 328 self._setup_subplots()
329 self._make_plot()
330 self._add_table()
File ~/.local/lib/python3.8/site-packages/pandas/plotting/_matplotlib/core.py:386, in MPLPlot._setup_subplots(self)
384 else:
385 if self.ax is None:
--> 386 fig = self.plt.figure(figsize=self.figsize)
387 axes = fig.add_subplot(111)
388 else:
File ~/.local/lib/python3.8/site-packages/matplotlib/pyplot.py:810, in figure(num, figsize, dpi, facecolor, edgecolor, frameon, FigureClass, clear, **kwargs)
798 _api.warn_external(
799 f"More than {max_open_warning} figures have been opened. "
800 f"Figures created through the pyplot interface "
(...)
803 f"warning, see the rcParam `figure.max_open_warning`).",
804 RuntimeWarning)
806 manager = new_figure_manager(
807 num, figsize=figsize, dpi=dpi,
808 facecolor=facecolor, edgecolor=edgecolor, frameon=frameon,
809 FigureClass=FigureClass, **kwargs)
--> 810 fig = manager.canvas.figure
811 if fig_label:
812 fig.set_label(fig_label)
AttributeError: 'NoneType' object has no attribute 'canvas'
AttributeError: 'NoneType' object has no attribute 'canvas'
I have checked the changes with git blame and saw that there is this line where the figure manager can turn None:
https://github.com/ipython/matplotlib-inline/blob/master/matplotlib_inline/backend_inline.py#L55 which has changed with commit 6380cd9.
Is it possible that this causes my issue? In my unittests, I use nbformat/nbconvert and for the docs I use nbsphinx. I am no expert in these tools myself but in each case just image files are generated and included in the output. They are in that sense not interactive. However, I still need the figure manager not to be None! A bit of discussion regarding this can be found at matplotlib/matplotlib#23648. There, it was mentioned that when the figure manager is None, something fishy is going on.
I am no expert in either tool. My first guess work, however, is that in 6380cd9 and later commits possibly some edge cases were not covered? I would love if you could have a look! Thank you!
Activity
anntzer commentedon Aug 17, 2022
Sorry, that return should have been a
return manager, I think. Not in front of my computer right now so feel free to make a pr out of the suggestion, or I'll do one when I can.Return manager instead of None
1kastner commentedon Aug 17, 2022
Thanks a lot, I have done as you have proposed!
Merge pull request #19 from 1kastner/patch-1
Release 0.1.6 to fix #18.
fperez commentedon Aug 18, 2022
Thx @1kastner for the fix! I just pushed 0.1.6 with this fix to PyPI, should be up on conda-forge soon.
Sorry @sryza for the hassles and thx for the prompt report.
1kastner commentedon Aug 18, 2022
@fperez thanks for the fast reaction and no worries. I just checked the repository and I was wondering whether there are any automated checks in place for the code? For example, unittests or Python type hints in combination with code linting might have helped to spot such an issue before release. I am just saying this because many, many people rely on this package.
Drop explicit installation of matplotlib-inline
Drop explicit installation of matplotlib-inline (#141)
fperez commentedon Aug 18, 2022
No, there aren't - yesterday I was very pressed for time and basically had to decide "do I spend the window I have adding some CI/tests to this, or do I finish this release quickly so the manual fix is out?" And I went for the latter, but given all my preaching about good testing, I felt bad about it :)
This package is tiny and moves very rarely, so in the past it hasn't been too terrible, but I 100% agree we should. Even putting in a few high-level tests in a notebook would be useful.
I'll try to find the time for that now that 0.1.6 is out and we're back where we needed to, but feel free to beat me to it!
1kastner commentedon Aug 19, 2022
I completely feel you! Some aspects can be tested better, such as "does this throw an exception in situations it should not", and some worse, such as "does it look beautiful". I have seen projects where they started a pixel-wise comparison of the expected and intended output. I believe humans are better in that task and it might be over the top for this project. But it is one important step to make the decision where to draw the line. I guess unittesting might be helpful but time-consuming as well.
If there is some testing infrastructure in place that the leading maintainers are all happy with, I could add one or two automated tests as well - probably some nbconvert invocations or something similar.