makemigrations serializer isinstance() check fails for subclasses of Decimal (and DateTime etc)

[oTree-org]

The migrations serializer uses isinstance(value, decimal.Decimal) to check if a value is a decimal:

​https://github.com/django/django/blob/ecb59cc6579402b68ddfd4499bf30edacf5963be/django/db/migrations/serializer.py#L363

If so, it adds from decimal import Decimal to the beginning of the migrations file:

​https://github.com/django/django/blob/ecb59cc6579402b68ddfd4499bf30edacf5963be/django/db/migrations/serializer.py#L87

The problem with isinstance() is that it results in false positives for subclasses. I am using a subclass of Decimal called Money as the default value for many of my model fields (subclass of DecimalField called MoneyField), and the migrations writer adds an incorrect from decimal import Decimal to my migrations files, which still results in a NameError for Money when I run migrate.

This problem also prevents me from using the deconstruct() method as recommended in the documentation to serialize custom classes:

​https://docs.djangoproject.com/ja/1.9/topics/migrations/#adding-a-deconstruct-method

(Because the check for Decimal happens before the check for .deconstruct().)

I think instead of isinstance(value, decimal.Decimal), the code should be if type(value) == decimal.Decimal.

[Tim Graham]

Maybe it would be better to move the if hasattr(value, 'deconstruct'): condition earlier in the checks?

[Markus Holtermann]

Doesn't warrant a backport as it's the same behavior on 1.7.

[Markus Holtermann <info@…>]

In 3b38308:

Fixed #26555 -- Gave deconstructible objects a higher priority during serialization