-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Bump minimum matplotlib version to 3.3.0 #7175
Conversation
In matplotlib 3.4.0 released on 03/26/2021 there were some deprecations around our use of Axes3d in the state visualization and bloch sphere modules. Fixing our usage to avoid these deprecations was previously attempted in Qiskit#6136 and Qiskit#6087 but we did not move forward there because the supported API we need to use matplotlib 3.3.0 when it was introduced. We previously did not want to raise our minimum supported matplotlib version to 3.3.0 to accomodate this. However, recent changes in transitive dependency of mpl, pyparsing, caused an incompatibility when running with older mpl versions around mathtex. To fix that we had to pin pyparsing in the constraints file (see Qiskit#7174). It appears that now is a better time to raise our minimum version because the burden of trying to keep support for older matplotlib versions is higher than it once was. This commit bumps the minimum matplotlib version to 3.3.0, updates the usage of Axes3d to avoid the deprecated usage in matplotlib 3.4.0, and removes the contraints pinning introduced in Qiskit#7174. This should get us on supported releases for matplotlib moving forward and it'll hopefully be a while before we encounter this kind of version issue in the future. Fixes Qiskit#6136
I was under the mistaken impression that the current mpl releases were compatible with pyparsing3, but looking at the linked mpl issue it's not fixed yet only for their development so far. So to unblock CI this restores the pinning as it's still needed until mpl releases a fix.
As far as I saw on the MPL GitHub (matplotlib/matplotlib#21445), they haven't fixed the problem yet, they just put the pin into their requirements, for 3.5 and (I think) backported to the 3.4 series. |
Yeah I saw that when ci came back failing, I reverted my change to the constraints file in: 228186f |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it expected that 20_plot_circuit_layout.png
would change in size (from 360x360px to 500x500px)?
I have no idea, I'd have to read through all the release notes to confirm. But changing matplotlib from 3.2.2 to 3.4.3 it did increase the size and it was something I could reproduce locally as well as in CI. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks ok to me now. The one particular image changing size is pretty weird, and marginally concerning, but at the end of the day, the output seems to be equivalent and generally I think people would just use these as quick visualisations, or do their own post-processing on them anyway.
"Skipping image comparison tests on python 3.6 as they " | ||
"depend on the local matplotlib environment matching the " | ||
"environment for the reference images which is only >=3.7", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of interest, what is the actual incompatible bit here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's something I had in #6087 IIRC it's because matplotlib 3.4.0 is for python>=3.7 so the reference image changes made when running with 3.4.0 isn't possible on python3.6 because we can't run with the latest matplotlib version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yeah, of course - I forgot that matplotlib might not have such a wide range of Python version support.
Summary
In matplotlib 3.4.0 released on 03/26/2021 there were some deprecations
around our use of Axes3d in the state visualization and bloch sphere
modules. Fixing our usage to avoid these deprecations was previously
attempted in #6136 and #6087 but we did not move forward there because
the supported API we need to use matplotlib 3.3.0 when it was
introduced. We previously did not want to raise our minimum supported
matplotlib version to 3.3.0 to accomodate this. However, recent changes
in transitive dependency of mpl, pyparsing, caused an incompatibility
when running with older mpl versions around mathtex. To fix that we
had to pin pyparsing in the constraints file (see #7174). It appears
that now is a better time to raise our minimum version because the
burden of trying to keep support for older matplotlib versions is higher
than it once was.
This commit bumps the minimum matplotlib version to 3.3.0, updates the
usage of Axes3d to avoid the deprecated usage in matplotlib 3.4.0, and
removes the contraints pinning introduced in #7174. This should get us
on supported releases for matplotlib moving forward and it'll hopefully
be a while before we encounter this kind of version issue in the future.
Details and comments
Fixes #6136