Output regression in 3.2 that affects SymPy's plotting

Bug report

Bug summary

The SymPy 1.5 documentation was built with matpltolib < 3.2, see the results here:

https://docs.sympy.org/latest/modules/plotting.html

Example image:

The SymPy development documentation (SymPy master branch) is now built with matplotlib 3.2, see the results here:

https://docs.sympy.org/dev/modules/plotting.html

Example image:

I've been documenting the issue over at SymPy here: sympy/sympy#19051

Code for reproduction

Note that SymPy has a wrapper around various plotting libraries and it will run matplotlib as the backend by default. I have not yet isolated what underlying mpl code in the SymPy wrapper is specifically affected.

from sympy import symbols
from sympy.plotting import plot
x = symbols('x')
p1 = plot(x*x, show=False)
p2 = plot(x, show=False)
p1.append(p2[0])
p1.show()

Actual outcome

# If applicable, paste the console output here
#
#

Expected outcome

Matplotlib version

Operating system: Ubuntu 18.04 and 19.10
Matplotlib version: 3.1 and 3.2
Matplotlib backend (print(matplotlib.get_backend())):
Python version: 3.6-3.8
Jupyter version (if applicable):
Other libraries: SymPy 1.5

Checked with MPL from conda-forge.

This comes from the postponing of autoscale calls. Previously, add_collection() (which is what sympy uses here) would trigger an autoscale (because it attempted to take markersize into consideration for margins computations), but now we made autoscaling lazier (which is faster, and consistent with add_line/add_image/..., and also including markersize into margins computation is fundamentally broken for reasons explained more in depth in the relevant PRs...); so sympy needs to add an ax.autoscale_view() call (I guess at the end of _process_series() but before forcing xlim/ylim).

This would be easier to understand if we can we boil this down to something that doesn't require sympy?

This would be easier to understand if we can we boil this down to something that doesn't require sympy?

Sure, hopefully someone will have some time (including me). We currently don't have a maintainer of our plotting package, so not some many people are familiar with the code.

@anntzer

Fair enough - it sounds like @anntzer looked into it enough to be able to put something together, so hopefully he can provide an example here. If not, I can try and take a look at it as well. Not sure if we can help w/ sympy ;-)

Well, I very roughly sketched a patch (add ax.autoscale_view() towards the end of _process_series), but won't have time to immediately dig more into that and check that it doesn't break anything else :)

See also #17016, which has, I think, a related issue where I was able to define a standalone example.

Yeah, that looks related. The plots on this page have similar effects:

https://docs.sympy.org/dev/modules/physics/continuum_mechanics/beam.html#module-sympy.physics.continuum_mechanics.beam

as compared to:

https://docs.sympy.org/latest/modules/physics/continuum_mechanics/beam.html#module-sympy.physics.continuum_mechanics.beam

@moorepants

@moorepants If you have the ability to run with development matplotlib, can you check #17017 on sympy? EDIT: can you check master matplotlib?

As commented in #17017, I'm pretty sure this is caused by #13593, which apparently was a deliberate change. (btw. #13593 also causes some hickups in #12518)

If you have the ability to run with development matplotlib, can you check #17017 on sympy? EDIT: can you check master matplotlib?

Sorry, day job is keeping me too busy. Maybe will have some time this weekend.

Trying with stable sympy it is still broken with Matplotlib master. Investigating now....

@ImportanceOfBeingErnest

Having now spent a while looking at this my question has moved from "why did this stop working?" to "why did this ever work?!"

I think @ImportanceOfBeingErnest has this right that #13593 is to blame. The change to add_collection in that PR was trying to maintain the old behavior, but apparently was not enough.

On a bit more digging, it seems that in 3.1.x autoscale_view was being called in set_xscale and set_yscale, but #15258 (backported as #15360). We now only trigger the auto-scale logic in set_*scale if the scale has changed enough that the "default limits" have changed. As you are changing the scale from linear -> linear that no longer triggers the autoscaling.

The xlimits look OK because sympy is explicitly setting it, but because the yscale logic is no longer being autoscaled so it stays as the default [0,1].

A minimal example that changes between 3.1 and 3.2 is

import numpy as np
import matplotlib.pyplot as plt
import matplotlib.collections as mcol

fig, ax = plt.subplots()

x = np.linspace(-10, 10, 100)
d = list(zip(x, x*x))

lc = mcol.LineCollection([d])
ax.add_collection(lc)
ax.set_xscale('linear')
ax.set_yscale('linear')

plt.show()

I think all of these changes that lead to this change of behavior were intentional so I'm not sure which parts of this we want to backout (falling back to view limits was to fix issue with datetime, which lead to an issues with log scale, which lead to this issue).

diff --git a/lib/matplotlib/axes/_base.py b/lib/matplotlib/axes/_base.py
index 694945d57..1d66e64c5 100644
--- a/lib/matplotlib/axes/_base.py
+++ b/lib/matplotlib/axes/_base.py
@@ -1819,6 +1819,7 @@ class _AxesBase(martist.Artist):
             # pre-lazy-autoscale behavior, which is not really better).
             self._unstale_viewLim()
             self.update_datalim(collection.get_datalim(self.transData))
+            self._request_autoscale_view()
 
         self.stale = True
         return collection

would fix sympy, but would change the behavior of just adding a collection. Now that we don't do the autoscaling immediately this is not a huge performance issue, but is a change in behavior so we should be sure that we are not going to cause further issues.

See #15008 for a “fix” to the datetime issues.

After digging into this, it seems to have been a coincidence that we were calling autoscale_view as part of set_scale rather than intended behavior. I don't think this is a regression that Matplotlib should fix, but sympy should merge sympy/sympy#19088 (or something like it) which explicitly triggers autoscaling when they want it to happen.

I am going to close this as sympy/sympy#19088 has been merged.

plotting: ensure autoscalling

Navigation Menu

Repository navigation

Output regression in 3.2 that affects SymPy's plotting #17004

Bug report

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Output regression in 3.2 that affects SymPy's plotting #17004

Description

Bug report

Activity

anntzer commented on Apr 2, 2020

jklymak commented on Apr 2, 2020

moorepants commented on Apr 2, 2020

jklymak commented on Apr 2, 2020

anntzer commented on Apr 2, 2020

mwaskom commented on Apr 3, 2020

moorepants commented on Apr 3, 2020

jklymak commented on Apr 3, 2020

ImportanceOfBeingErnest commented on Apr 3, 2020

moorepants commented on Apr 3, 2020

tacaswell commented on Apr 3, 2020

tacaswell commented on Apr 4, 2020

tacaswell commented on Apr 7, 2020

jklymak commented on Apr 7, 2020

tacaswell commented on Apr 16, 2020

tacaswell commented on Jun 10, 2020

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions