From 93fc0110c7c92ff9531c1373d98107f250450bd1 Mon Sep 17 00:00:00 2001 From: Mike Fogel Date: Tue, 8 Feb 2022 14:34:27 -0800 Subject: [PATCH] Streamline serialization for migrations Trying to avoid uncessary creation of migrations. --- tests/test_deconstruct.py | 161 +++++++++++++++++++------------------- timezone_field/fields.py | 43 +++++----- 2 files changed, 97 insertions(+), 107 deletions(-) diff --git a/tests/test_deconstruct.py b/tests/test_deconstruct.py index f588a4c..2a9614d 100644 --- a/tests/test_deconstruct.py +++ b/tests/test_deconstruct.py @@ -5,70 +5,40 @@ from timezone_field import TimeZoneField from timezone_field.compat import ZoneInfo - -@pytest.fixture -def fields(use_pytz): - return ( - TimeZoneField(use_pytz=use_pytz), - TimeZoneField(default="UTC", use_pytz=use_pytz), - TimeZoneField(max_length=42, use_pytz=use_pytz), - TimeZoneField( - choices=[ - (pytz.timezone("US/Pacific"), "US/Pacific"), - (pytz.timezone("US/Eastern"), "US/Eastern"), - ], - use_pytz=use_pytz, - ), - TimeZoneField( - choices=[ - (pytz.timezone(b"US/Pacific"), b"US/Pacific"), - (pytz.timezone(b"US/Eastern"), b"US/Eastern"), - ], - use_pytz=use_pytz, - ), - TimeZoneField( - choices=[ - ("US/Pacific", "US/Pacific"), - ("US/Eastern", "US/Eastern"), - ], - use_pytz=use_pytz, - ), - TimeZoneField( - choices=[ - (b"US/Pacific", b"US/Pacific"), - (b"US/Eastern", b"US/Eastern"), - ], - use_pytz=use_pytz, - ), - ) - - -def test_deconstruct(fields, use_pytz): - if not use_pytz: - fields += ( - TimeZoneField( - choices=[ - (ZoneInfo("US/Pacific"), "US/Pacific"), - (ZoneInfo("US/Eastern"), "US/Eastern"), - ], - use_pytz=use_pytz, - ), - ) - for org_field in fields: - _name, _path, args, kwargs = org_field.deconstruct() - new_field = TimeZoneField(*args, use_pytz=use_pytz, **kwargs) - assert org_field.max_length == new_field.max_length - assert org_field.choices == new_field.choices - - -def test_full_serialization(fields): +test_fields = [ + TimeZoneField(), + TimeZoneField(default="UTC"), + TimeZoneField(max_length=42), + TimeZoneField(use_pytz=True), + TimeZoneField(use_pytz=False), + TimeZoneField(choices=[("US/Pacific", "US/Pacific"), ("US/Eastern", "US/Eastern")]), + TimeZoneField(choices=[(b"US/Pacific", b"US/Pacific"), (b"US/Eastern", b"US/Eastern")]), + TimeZoneField( + choices=[(pytz.timezone("US/Pacific"), "US/Pacific"), (pytz.timezone("US/Eastern"), "US/Eastern")], + use_pytz=True, + ), + TimeZoneField( + choices=[(ZoneInfo("US/Pacific"), "US/Pacific"), (ZoneInfo("US/Eastern"), "US/Eastern")], + use_pytz=False, + ), +] + + +@pytest.mark.parametrize("field", test_fields) +def test_deconstruct(field): + _name, _path, args, kwargs = field.deconstruct() + new_field = TimeZoneField(*args, **kwargs) + assert field.max_length == new_field.max_length + assert field.choices == new_field.choices + + +@pytest.mark.parametrize("field", test_fields) +def test_full_serialization(field): # ensure the values passed to kwarg arguments can be serialized # the recommended 'deconstruct' testing by django docs doesn't cut it # https://docs.djangoproject.com/en/1.7/howto/custom-model-fields/#field-deconstruction # replicates https://github.com/mfogel/django-timezone-field/issues/12 - for field in fields: - # ensuring the following call doesn't throw an error - MigrationWriter.serialize(field) + MigrationWriter.serialize(field) # should not throw def test_from_db_value(use_pytz): @@ -82,51 +52,64 @@ def test_from_db_value(use_pytz): assert utc == value -def test_default_kwargs_not_frozen(use_pytz): +def test_default_kwargs_not_frozen(): """ Ensure the deconstructed representation of the field does not contain kwargs if they match the default. Don't want to bloat everyone's migration files. """ - field = TimeZoneField(use_pytz=use_pytz) + field = TimeZoneField() _name, _path, _args, kwargs = field.deconstruct() assert "choices" not in kwargs assert "max_length" not in kwargs -def test_specifying_defaults_not_frozen(use_pytz, tz_func): +def test_specifying_defaults_not_frozen(): """ If someone's matched the default values with their kwarg args, we - shouldn't bothering freezing those. + shouldn't bothering freezing those + (excluding the use_pytz, which changes with your django version). """ - field = TimeZoneField(max_length=63, use_pytz=use_pytz) + field = TimeZoneField(max_length=63) _name, _path, _args, kwargs = field.deconstruct() assert "max_length" not in kwargs - choices = [(tz_func(tz), tz.replace("_", " ")) for tz in pytz.common_timezones] - field = TimeZoneField(choices=choices, use_pytz=use_pytz) + choices = [(tz, tz.replace("_", " ")) for tz in pytz.common_timezones] + field = TimeZoneField(choices=choices) _name, _path, _args, kwargs = field.deconstruct() assert "choices" not in kwargs - choices = [(tz, tz.replace("_", " ")) for tz in pytz.common_timezones] - field = TimeZoneField(choices=choices, use_pytz=use_pytz) + choices = [(pytz.timezone(tz), tz.replace("_", " ")) for tz in pytz.common_timezones] + field = TimeZoneField(choices=choices, use_pytz=True) + _name, _path, _args, kwargs = field.deconstruct() + assert "choices" not in kwargs + + choices = [(ZoneInfo(tz), tz.replace("_", " ")) for tz in pytz.common_timezones] + field = TimeZoneField(choices=choices, use_pytz=False) _name, _path, _args, kwargs = field.deconstruct() assert "choices" not in kwargs @pytest.mark.parametrize( - "use_tz_object, timezones", + "choices, use_pytz", [ - [True, ["US/Pacific", "US/Eastern"]], - [False, ["US/Pacific", "US/Eastern"]], + [ + [("US/Pacific", "US/Pacific"), ("US/Eastern", "US/Eastern")], + None, + ], + [ + [(pytz.timezone("US/Pacific"), "US/Pacific"), (pytz.timezone("US/Eastern"), "US/Eastern")], + True, + ], + [ + [(ZoneInfo("US/Pacific"), "US/Pacific"), (ZoneInfo("US/Eastern"), "US/Eastern")], + False, + ], ], ) -def test_deconstruct_when_using_just_choices(use_tz_object, timezones, use_pytz, tz_func): - if not use_tz_object: - tz_func = str - choices = [(tz_func(tz), tz) for tz in timezones] - field = TimeZoneField(choices=choices, use_pytz=use_pytz) - _name, _path, _args, kwargs = field.deconstruct() +def test_deconstruct_when_using_choices(choices, use_pytz): + field = TimeZoneField(choices=choices) + _name, _path, args, kwargs = field.deconstruct() assert kwargs == { "choices": [ ("US/Pacific", "US/Pacific"), @@ -143,8 +126,8 @@ def test_deconstruct_when_using_just_choices(use_tz_object, timezones, use_pytz, ["WITH_GMT_OFFSET", {"choices_display": "WITH_GMT_OFFSET"}], ], ) -def test_deconstruct_when_using_just_choices_display(use_pytz, choices_display, expected_kwargs): - field = TimeZoneField(choices_display=choices_display, use_pytz=use_pytz) +def test_deconstruct_when_using_choices_display(choices_display, expected_kwargs): + field = TimeZoneField(choices_display=choices_display) _name, _path, _args, kwargs = field.deconstruct() assert kwargs == expected_kwargs @@ -174,7 +157,21 @@ def test_deconstruct_when_using_just_choices_display(use_pytz, choices_display, ], ], ) -def test_deconstruct_when_using_choices_and_choices_display(use_pytz, choices, choices_display, expected_kwargs): - field = TimeZoneField(choices=choices, choices_display=choices_display, use_pytz=use_pytz) +def test_deconstruct_when_using_choices_and_choices_display(choices, choices_display, expected_kwargs): + field = TimeZoneField(choices=choices, choices_display=choices_display) + _name, _path, _args, kwargs = field.deconstruct() + assert kwargs == expected_kwargs + + +@pytest.mark.parametrize( + "use_pytz, expected_kwargs", + [ + [None, {}], + [True, {"use_pytz": True}], + [False, {"use_pytz": False}], + ], +) +def test_deconstruct_when_using_use_pytz(use_pytz, expected_kwargs): + field = TimeZoneField(use_pytz=use_pytz) _name, _path, _args, kwargs = field.deconstruct() assert kwargs == expected_kwargs diff --git a/timezone_field/fields.py b/timezone_field/fields.py index 9ab8727..5a5b21a 100644 --- a/timezone_field/fields.py +++ b/timezone_field/fields.py @@ -7,11 +7,6 @@ from timezone_field.compat import ZoneInfo, ZoneInfoNotFoundError from timezone_field.utils import is_pytz_instance, use_pytz_default -default_pytz_tzs = [pytz.timezone(tz) for tz in pytz.common_timezones] -default_pytz_choices = standard(default_pytz_tzs) -default_zoneinfo_tzs = [ZoneInfo(tz) for tz in pytz.common_timezones] -default_zoneinfo_choices = standard(default_zoneinfo_tzs) - class TimeZoneField(models.Field): """ @@ -42,6 +37,8 @@ class TimeZoneField(models.Field): # NOTE: these defaults are excluded from migrations. If these are changed, # existing migration files will need to be accomodated. default_max_length = 63 + default_pytz_tzs = [pytz.timezone(tz) for tz in pytz.common_timezones] + default_zoneinfo_tzs = [ZoneInfo(tz) for tz in pytz.common_timezones] def __init__(self, *args, **kwargs): # allow some use of positional args up until the args we customize @@ -49,11 +46,11 @@ def __init__(self, *args, **kwargs): # https://github.com/django/django/blob/1.11.11/django/db/models/fields/__init__.py#L145 if len(args) > 3: raise ValueError("Cannot specify max_length by positional arg") - kwargs.setdefault("max_length", self.default_max_length) - self.use_pytz = kwargs.pop("use_pytz", use_pytz_default()) - self.default_tzs = default_pytz_tzs if self.use_pytz else default_zoneinfo_tzs - self.default_choices = default_pytz_choices if self.use_pytz else default_zoneinfo_choices + self.max_length = kwargs.pop("max_length", self.default_max_length) + self.use_pytz_explicit = kwargs.pop("use_pytz", None) + self.use_pytz = self.use_pytz_explicit if self.use_pytz_explicit is not None else use_pytz_default() + self.default_tzs = self.default_pytz_tzs if self.use_pytz else self.default_zoneinfo_tzs if "choices" in kwargs: values, displays = zip(*kwargs["choices"]) @@ -100,26 +97,22 @@ def deconstruct(self): if kwargs.get("max_length") == self.default_max_length: del kwargs["max_length"] + if self.use_pytz_explicit is not None: + kwargs["use_pytz"] = self.use_pytz_explicit + if self.choices_display is not None: kwargs["choices_display"] = self.choices_display - choices = kwargs.get("choices") - if choices: - if self.choices_display is None: - defaults = [default_pytz_choices] - if default_zoneinfo_tzs: - defaults += [default_zoneinfo_choices] - if choices == self.default_choices: - kwargs.pop("choices") + choices = kwargs["choices"] + if self.choices_display is None: + if choices == standard(self.default_tzs): + kwargs.pop("choices") + else: + values, _ = zip(*choices) + if sorted(values, key=str) == sorted(self.default_tzs, key=str): + kwargs.pop("choices") else: - values, _ = zip(*choices) - defaults = [default_pytz_tzs] - if default_zoneinfo_tzs: - defaults += [default_zoneinfo_tzs] - if sorted(values, key=str) in defaults: - kwargs.pop("choices") - else: - kwargs["choices"] = [(value, "") for value in values] + kwargs["choices"] = [(value, "") for value in values] # django can't decontruct pytz objects, so transform choices # to [, ] format for writing out to the migration