paulocheque / django-dynamic-fixture

A complete library to create dynamic model instances for testing purposes.
http://django-dynamic-fixture.readthedocs.io/
Other
391 stars 67 forks source link

Prevent deprecation warnings for distutils StrictVersion when running with setuptools or Python >= 3.10 #150

Closed PoByBolek closed 2 years ago

PoByBolek commented 2 years ago

Using distutils.version.StrictVersion raises a deprecation warning when running with setuptools >= 59.6.0 (2021-12-12) or Python >= 3.10 (PEP 632).

So instead of messing around with regular expressions and StrictVersion this pull request numerically compares the first two version parts of django.VERSION, which has been around since Django 0.9.1 (2006-02-17) and seems to always have at least to integer components.

However, this will break clients that use django_greater_than() themselves (is this part of DDF's public interface?). If this is a concern, I could check if the major argument is a string and attempt to split it into integers, maybe raising another deprecation warning in the process.

Alternatively, I could try to import packaging.version.Version and fall back to distutils.version.StrictVersion if packaging cannot be imported. But I really like the numeric approach here :)

JanMalte commented 2 years ago

Thank you very much for your contribution.

I would suggest to use packaging.version.Version as noted in the migration advice for PEP 632: https://peps.python.org/pep-0632/#migration-advice

PoByBolek commented 2 years ago

Sure thing!

Using packaging.version.Version would mean adding an install time dependency to packaging in setup.py though, right?

Could you explain your reasoning for preferring Version(django.get_version()) over django.VERSION please (besides the migration advice in PEP 632)? I mean, django.get_version() ultimately depends on django.VERSION and I feel llike simply comparing two numeric tuples is much more straight forward than parsing version strings and comparing those.

paulocheque commented 2 years ago

Awesome!

ps: it is always preferrable to avoid new dependencies, specially for simple things like this. I will merge for now.