Repository navigation
Output regression in 3.2 that affects SymPy's plotting #17004
Description
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.
Activity
anntzer commented on Apr 2, 2020
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).
jklymak commented on Apr 2, 2020
This would be easier to understand if we can we boil this down to something that doesn't require sympy?
moorepants commented on Apr 2, 2020
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.
jklymak commented on Apr 2, 2020
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 ;-)
anntzer commented on Apr 2, 2020
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 :)
mwaskom commented on Apr 3, 2020
See also #17016, which has, I think, a related issue where I was able to define a standalone example.
moorepants commented on Apr 3, 2020
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
jklymak commented on Apr 3, 2020
@moorepants If you have the ability to run with development matplotlib, can you check #17017 on sympy? EDIT: can you check master matplotlib?
ImportanceOfBeingErnest commented on Apr 3, 2020
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)
moorepants commented on Apr 3, 2020
Sorry, day job is keeping me too busy. Maybe will have some time this weekend.
tacaswell commented on Apr 3, 2020
Trying with stable sympy it is still broken with Matplotlib master. Investigating now....
tacaswell commented on Apr 4, 2020
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.
tacaswell commented on Apr 7, 2020
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
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).
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.
jklymak commented on Apr 7, 2020
See #15008 for a “fix” to the datetime issues.
tacaswell commented on Apr 16, 2020
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.
tacaswell commented on Jun 10, 2020
I am going to close this as sympy/sympy#19088 has been merged.
plotting: ensure autoscalling