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

Fix using matplotlib._all_deprecated, which is deprecated itself! #44

Merged
merged 2 commits into from
Nov 21, 2021

Conversation

ericpre
Copy link
Contributor

@ericpre ericpre commented Nov 3, 2021

See matplotlib/matplotlib#21004.

from matplotlib_scalebar import scalebar will not work with the coming matplotlib 3.5 release.

@ericpre
Copy link
Contributor Author

ericpre commented Nov 17, 2021

@ppinard, matplotlib 3.5 has been released and updating matplotlib breaks matplotlib-scalebar, because of the use of matplotlib private API in matplotlib-scalebar.

@hakonanes
Copy link

This would affect orix (and dependent packages pyxem, kikuchipy, diffsims) as well. But, I don't actually get an error when doing from matplotlib_scalebar import scalebar with matplotlib==3.5.0, and it seems like _all_deprecated is still around in 3.5.0, since this works fine

>>> from matplotlib import _all_deprecated, __version__
>>> print(__version__)
3.5.0

@ericpre
Copy link
Contributor Author

ericpre commented Nov 18, 2021

Sorry for the confusion, this is still on matplotlib main and will be release in 3.6 - since the PR was merged more than two month ago I wrongly assume, it would be in the 3.5 release.

@ppinard: any idea when you could make a release fixing this issue? The fix is simple and the issue is quite annoying! Thanks!

@hakonanes
Copy link

Thanks for the clarification! +1 for seeing a patch release :)

@@ -111,11 +112,17 @@ def _validate_legend_loc(loc):
}
)


if Version(matplotlib.__version__ ) >= Version('3.5.dev'):
Copy link
Owner

Choose a reason for hiding this comment

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

I'm just wondering whether this would be simpler to avoid the dependency to the packaging module:

_all_deprecated = getattr(matplotlib, "_all_deprecated", {})

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is better! :)

@ppinard
Copy link
Owner

ppinard commented Nov 21, 2021

Thanks @ericpre for the PR and my apologies for not replying sooner.

@ppinard
Copy link
Owner

ppinard commented Nov 21, 2021

In #45 I added a new CI build that tests matplotlib-scalebar against the main branch of matplotlib. I'll merge it after this PR is completed to catch any future issues.

@ppinard ppinard merged commit 2c0f5ad into ppinard:master Nov 21, 2021
"Topic :: Scientific/Engineering :: Visualization",
],
packages=find_packages(),
package_data={},
python_requires='~=3.7',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure if this is necessary in this case, because matplotlib 3.5 dropped support for python 3.6, but using getattr(matplotlib, "_all_deprecated", {}) should require python 3.7 (https://www.python.org/dev/peps/pep-0562). I haven't checked if this is correct but in any case, I think that it doesn't hurt to be explicit on the minimum python version supported!

Copy link
Owner

Choose a reason for hiding this comment

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

Actually getattr was there since Python 2.x (https://docs.python.org/2.7/library/functions.html#getattr) but there is little point that matplotlib-scalebar supports Python versions that matplotlib doesn't! Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants