Skip to content

Use packaging to do version comparisons. #20437

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

Merged
merged 1 commit into from
Jun 19, 2021
Merged

Conversation

QuLogic
Copy link
Member

@QuLogic QuLogic commented Jun 15, 2021

PR Summary

... as distutils is deprecated in Python 3.10.

Since this is somewhat lower-level, I picked a minimum version on the same timescale as Python support, namely about 3 years ago. Minimum version is ~12 months ago, since this is a Python-only package and all relevant downstreams appear to have it.

Maybe this should get an API note or something?

PR Checklist

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (run flake8 on changed files to check).
  • [n/a] New features are documented, with examples if plot related.
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • Conforms to Matplotlib style conventions (install flake8-docstrings and run flake8 --docstring-convention=all).
  • [n/a] New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • [n/a] API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).

# https://github.com/ImageMagick/ImageMagick/issues/2720
raise ExecutableNotFoundError(
f"You have ImageMagick {info.version}, which is unsupported")
return info
elif name == "pdftops":
info = impl(["pdftops", "-v"], "^pdftops version (.*)",
ignore_exit_code=True)
if info and not ("3.0" <= info.version
# poppler version numbers.
or "0.9" <= info.version <= "1.0"):
Copy link
Member Author

Choose a reason for hiding this comment

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

Note the <= 1.0 was an incorrect conversion from 32ed5bd

@QuLogic
Copy link
Member Author

QuLogic commented Jun 15, 2021

Hmm, guess I need to actually fix compatibility with old packaging, if that's the version we want to support.

@anntzer
Copy link
Contributor

anntzer commented Jun 15, 2021

looks like you either need to replace foo.major < 1 by foo < parse_version("1") or just depend on packaging>=20.0 (which introduced .major); the latter's fine too per the minimum versions policy (no compiled component == 12 months).

@tacaswell tacaswell added this to the v3.5.0 milestone Jun 17, 2021
@QuLogic
Copy link
Member Author

QuLogic commented Jun 17, 2021

I looked through downstream distributions and the ones that are at least close to latest Matplotlib have at least packaging 20, so I bumped the version instead of trying to work out the changes needed (not that there were many.)

... as `distutils` is deprecated in Python 3.10.

The minimum is ~a year ago, which appears to be available in all distros
that are close to current in Matplotlib.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants