CarliJoy / django-pint

Simple django app for storing Quantities with units, leveraging the Pint library
https://django-pint.readthedocs.io
MIT License
39 stars 16 forks source link

Can't adapt type Quantity when using DecimalQuantityField #36

Closed CodyMorton closed 2 years ago

CodyMorton commented 2 years ago

Hi, I've been trying to implement this into a new Django app but I was having a lot of trouble saving new models using the DecimalQuantityField. QuantityField by itself works fine.

After a bit of debugging, I noticed a Quantity object was being passed to the django SQLInsertCompiler. I'm new to Django so not sure if this is the right place to be looking. Anyways, this is because in fields.py DecimalQuantityField, this method returns a Quantity() object:

    def get_db_prep_save(self, value, connection):
        """
        Get Value that shall be saved to database, make sure it is transformed
        """
        value = self.get_prep_value(value)
        return super().get_db_prep_save(value, connection)

I changed it to this:

    def get_db_prep_save(self, value, connection):
        """
        Get Value that shall be saved to database, make sure it is transformed
        """
        value = self.get_prep_value(value)
        return value

And it fixed my issue! The super().get_db_prep_save() was going all the way up to django.db.models.fields.DecimalField. Surely this isn't correct?

I would fork and submit a pull request, but I wanted to make sure I am understanding / fixing the issue correctly. If my change is correct, let me know and I can do a pull request.

Thank you so much!

Dependencies: asgiref==3.4.1 Django==3.2.8 django-environ==0.8.1 gunicorn==20.1.0 psycopg2-binary==2.9.1 pytz==2021.3 sqlparse==0.4.2 typing-extensions==3.10.0.2 django-bootstrap-v5==1.0.6 fontawesomefree==5.15.4 django-pint==0.6.2

Using postgres!

CarliJoy commented 2 years ago

Hi,

I know a bigger project that uses DecimalQuantityField just fine. The super call is required for sure! Indeed it has to perform all the DecimalField transformations, as the Quantity part only takes care to add/remove the "pint" part (which represents the quantity). Did the test run through after you deleted the suggested line?

Have you tried using normal DecimalFields ? Do your problem occur there as well?

Maybe also describe your problems a bit more in detail...

CodyMorton commented 2 years ago

Ugh, so sorry then. Thanks for your patience. Do you think you could help me debug?

I did test this and it worked, I noticed that within the parameters it was sending to the SQL insert command were Quantity<> objects. Is this correct? After removing the line it was sending Decimal objects, which worked.

I've been trying to figure this out for a bit. All I have are two models and one uses DecimalQuantityField. Again, QuantityField works fine. When I try to use the admin forms to create a new object OR use my own forms, I get the error below. I've tried a lot of things, and the only thing that has ever worked was removing that line as I described above.

I have tried using QuantityField (float) instead, which does work. I have also tried making my own forms and submitting the data through that. This has the same result as the admin panel's form. I've tried moving around the quantityfield in the list of installed apps, and also trying debugging psycopg2 but it does seem to be working. And yes, I have tried regular DecimalFields and those work fine.

Do you see anything I am doing particularly wrong? Thank you so much for your help!

models.py:

import decimal
from django.db import models
from django.core.validators import MaxValueValidator, MinValueValidator
from django.db.models import base
from django.db.models.deletion import PROTECT
from django.db.models.fields import DecimalField
from pint import unit
from my_project import settings
from django.utils.timezone import now
import math
from decimal import *

# https://github.com/CarliJoy/django-pint
# use DecimalQuantityField for fields that need to support multiple units
# for a list of units, see https://github.com/hgrecco/pint/blob/master/pint/default_en.txt
from quantityfield.fields import DecimalQuantityField
# pull in the quantity field so we can pass defaults to the model
from my_project.settings import DJANGO_PINT_UNIT_REGISTER as ureg
Quantity = ureg.Quantity

# Create your models here.

# this class specifies basic info we want on most objects
# to use this, make the superclass CommonObjectInfo instead of models.Model
# if you modify this it modifies all of the sub classes as well
class CommonObjectInfo(models.Model):
    name = models.CharField(max_length=100)
    description = models.CharField("description",max_length = 250)
    modifiedDate = models.DateField("date last modified", default=now, editable=False)
    createdDate = models.DateField("date last created", default=now, editable=False)

    def __str__(self):
      return self.name

    class Meta:
        abstract = True

class Structure(CommonObjectInfo):
  user = models.ForeignKey(
        settings.AUTH_USER_MODEL,
        on_delete=models.CASCADE,
        verbose_name= "structure creator"
    )
  publicBool = models.BooleanField("Public", default=False)
  protection_potential = DecimalQuantityField(verbose_name="protection potential", default=Decimal("-0.8"), decimal_places=4, max_digits=8, base_units="volt")
  waterdepth = DecimalQuantityField(verbose_name="water depth", null=True, decimal_places=4, max_digits=24, base_units="meter")
  designlife_yr = DecimalQuantityField(verbose_name="structure service life", null=True, decimal_places=4, max_digits=8, base_units="year")

# Main project class which manages the design of a project.
class Project(CommonObjectInfo):
  user = models.ForeignKey(
        settings.AUTH_USER_MODEL, on_delete=models.CASCADE,
        verbose_name= "project creator"
    )
  structures = models.ManyToManyField(Structure, related_name="projects", blank=True)

settings.py (database excluded) the database engine is set to: 'ENGINE': 'django.db.backends.postgresql_psycopg2'

"""
Django settings for my_project project.

Generated by 'django-admin startproject' using Django 3.2.8.

For more information on this file, see
https://docs.djangoproject.com/en/3.2/topics/settings/

For the full list of settings and their values, see
https://docs.djangoproject.com/en/3.2/ref/settings/
"""
import os
from pathlib import Path

# Build paths inside the project like this: BASE_DIR / 'subdir'.
BASE_DIR = Path(__file__).resolve().parent.parent

# Quick-start development settings - unsuitable for production
# See https://docs.djangoproject.com/en/3.2/howto/deployment/checklist/

# SECURITY WARNING: keep the secret key used in production secret!
SECRET_KEY = #removed

# SECURITY WARNING: don't run with debug turned on in production!
DEBUG = True

ALLOWED_HOSTS = #removed

# Application definition

INSTALLED_APPS = [
    'ProjectManager',
    'bootstrap5',
    'django.contrib.admin',
    'django.contrib.auth',
    'django.contrib.contenttypes',
    'django.contrib.sessions',
    'django.contrib.messages',
    'django.contrib.staticfiles',
    'django.contrib.postgres',
    'fontawesomefree',
    "quantityfield"
]

MIDDLEWARE = [
    'django.middleware.security.SecurityMiddleware',
    'django.contrib.sessions.middleware.SessionMiddleware',
    'django.middleware.common.CommonMiddleware',
    'django.middleware.csrf.CsrfViewMiddleware',
    'django.contrib.auth.middleware.AuthenticationMiddleware',
    'django.contrib.messages.middleware.MessageMiddleware',
    'django.middleware.clickjacking.XFrameOptionsMiddleware',
    'my_project.middleware.LoginRequiredMiddleware'
]

ROOT_URLCONF = 'my_project.urls'

TEMPLATES = [
    {
        'BACKEND': 'django.template.backends.django.DjangoTemplates',
        'DIRS': [
            os.path.join(BASE_DIR, 'templates'),
        ],
        'APP_DIRS': True,
        'OPTIONS': {
            'context_processors': [
                'django.template.context_processors.debug',
                'django.template.context_processors.request',
                'django.contrib.auth.context_processors.auth',
                'django.contrib.messages.context_processors.messages',
            ],
        },
    },
]

WSGI_APPLICATION = 'my_project.wsgi.application'

# Password validation
# https://docs.djangoproject.com/en/3.2/ref/settings/#auth-password-validators

AUTH_PASSWORD_VALIDATORS = [
    {
        'NAME': 'django.contrib.auth.password_validation.UserAttributeSimilarityValidator',
    },
    {
        'NAME': 'django.contrib.auth.password_validation.MinimumLengthValidator',
    },
    {
        'NAME': 'django.contrib.auth.password_validation.CommonPasswordValidator',
    },
    {
        'NAME': 'django.contrib.auth.password_validation.NumericPasswordValidator',
    },
]

# Internationalization
# https://docs.djangoproject.com/en/3.2/topics/i18n/

LANGUAGE_CODE = 'en-us'

TIME_ZONE = 'UTC'

USE_I18N = True

USE_L10N = True

USE_TZ = True

# Static files (CSS, JavaScript, Images)
# https://docs.djangoproject.com/en/3.2/howto/static-files/

STATIC_URL = '/static/'

STATIC_ROOT = os.path.join(BASE_DIR, 'static/')

# Default primary key field type
# https://docs.djangoproject.com/en/3.2/ref/settings/#default-auto-field

DEFAULT_AUTO_FIELD = 'django.db.models.BigAutoField'

# Changing login redirect

LOGIN_REDIRECT_URL = "Home"
LOGOUT_REDIRECT_URL = "Home"

# These are URLS that are except from a required login
AUTH_EXEMPT_ROUTES = ('Register', 'login', 'password_reset')
AUTH_LOGIN_ROUTE = 'login'

# Setting the default user model, for now we are using the model provided by Django
AUTH_USER_MODEL = 'auth.User'

# Email settings - These will need to be setup for production seperately
EMAIL_HOST = "localhost"
EMAIL_PORT = 1025

# django-pint settings
from pint import UnitRegistry
# django-pint will set the DJANGO_PINT_UNIT_REGISTER automatically
# as application_registry
default_ureg = UnitRegistry()
default_ureg.load_definitions('my_project/units.txt')
DJANGO_PINT_UNIT_REGISTER = default_ureg

Error:

Environment:

Request Method: POST
Request URL: http://localhost:8000/admin/ProjectManager/structure/1/change/

Django Version: 3.2.8
Python Version: 3.9.7
Installed Applications:
['ProjectManager',
 'bootstrap5',
 'django.contrib.admin',
 'django.contrib.auth',
 'django.contrib.contenttypes',
 'django.contrib.sessions',
 'django.contrib.messages',
 'django.contrib.staticfiles',
 'django.contrib.postgres',
 'fontawesomefree',
 'quantityfield']
Installed Middleware:
['django.middleware.security.SecurityMiddleware',
 'django.contrib.sessions.middleware.SessionMiddleware',
 'django.middleware.common.CommonMiddleware',
 'django.middleware.csrf.CsrfViewMiddleware',
 'django.contrib.auth.middleware.AuthenticationMiddleware',
 'django.contrib.messages.middleware.MessageMiddleware',
 'django.middleware.clickjacking.XFrameOptionsMiddleware',
 'my_project.middleware.LoginRequiredMiddleware']

Traceback (most recent call last):
  File "c:\Users\Cody\Desktop\374 Dev\my_project\my_projectEnv\lib\site-packages\django\db\backends\utils.py", line 84, in _execute
    return self.cursor.execute(sql, params)

The above exception (can't adapt type 'Quantity') was the direct cause of the following exception:
  File "c:\Users\Cody\Desktop\374 Dev\my_project\my_projectEnv\lib\site-packages\django\core\handlers\exception.py", line 47, in inner
    response = get_response(request)
  File "c:\Users\Cody\Desktop\374 Dev\my_project\my_projectEnv\lib\site-packages\django\core\handlers\base.py", line 181, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "c:\Users\Cody\Desktop\374 Dev\my_project\my_projectEnv\lib\site-packages\django\contrib\admin\options.py", line 616, in wrapper
    return self.admin_site.admin_view(view)(*args, **kwargs)
  File "c:\Users\Cody\Desktop\374 Dev\my_project\my_projectEnv\lib\site-packages\django\utils\decorators.py", line 130, in _wrapped_view
    response = view_func(request, *args, **kwargs)
  File "c:\Users\Cody\Desktop\374 Dev\my_project\my_projectEnv\lib\site-packages\django\views\decorators\cache.py", line 44, in _wrapped_view_func
    response = view_func(request, *args, **kwargs)
  File "c:\Users\Cody\Desktop\374 Dev\my_project\my_projectEnv\lib\site-packages\django\contrib\admin\sites.py", line 232, in inner
    return view(request, *args, **kwargs)
  File "c:\Users\Cody\Desktop\374 Dev\my_project\my_projectEnv\lib\site-packages\django\contrib\admin\options.py", line 1660, in change_view
    return self.changeform_view(request, object_id, form_url, extra_context)
  File "c:\Users\Cody\Desktop\374 Dev\my_project\my_projectEnv\lib\site-packages\django\utils\decorators.py", line 43, in _wrapper
    return bound_method(*args, **kwargs)
  File "c:\Users\Cody\Desktop\374 Dev\my_project\my_projectEnv\lib\site-packages\django\utils\decorators.py", line 130, in _wrapped_view
    response = view_func(request, *args, **kwargs)
  File "c:\Users\Cody\Desktop\374 Dev\my_project\my_projectEnv\lib\site-packages\django\contrib\admin\options.py", line 1540, in changeform_view
    return self._changeform_view(request, object_id, form_url, extra_context)
  File "c:\Users\Cody\Desktop\374 Dev\my_project\my_projectEnv\lib\site-packages\django\contrib\admin\options.py", line 1586, in _changeform_view
    self.save_model(request, new_object, form, not add)
  File "c:\Users\Cody\Desktop\374 Dev\my_project\my_projectEnv\lib\site-packages\django\contrib\admin\options.py", line 1099, in save_model
    obj.save()
  File "c:\Users\Cody\Desktop\374 Dev\my_project\my_projectEnv\lib\site-packages\django\db\models\base.py", line 726, in save
    self.save_base(using=using, force_insert=force_insert,
  File "c:\Users\Cody\Desktop\374 Dev\my_project\my_projectEnv\lib\site-packages\django\db\models\base.py", line 763, in save_base
    updated = self._save_table(
  File "c:\Users\Cody\Desktop\374 Dev\my_project\my_projectEnv\lib\site-packages\django\db\models\base.py", line 845, in _save_table
    updated = self._do_update(base_qs, using, pk_val, values, update_fields,
  File "c:\Users\Cody\Desktop\374 Dev\my_project\my_projectEnv\lib\site-packages\django\db\models\base.py", line 899, in _do_update
    return filtered._update(values) > 0
  File "c:\Users\Cody\Desktop\374 Dev\my_project\my_projectEnv\lib\site-packages\django\db\models\query.py", line 802, in _update
    return query.get_compiler(self.db).execute_sql(CURSOR)
  File "c:\Users\Cody\Desktop\374 Dev\my_project\my_projectEnv\lib\site-packages\django\db\models\sql\compiler.py", line 1559, in execute_sql
    cursor = super().execute_sql(result_type)
  File "c:\Users\Cody\Desktop\374 Dev\my_project\my_projectEnv\lib\site-packages\django\db\models\sql\compiler.py", line 1175, in execute_sql
    cursor.execute(sql, params)
  File "c:\Users\Cody\Desktop\374 Dev\my_project\my_projectEnv\lib\site-packages\django\db\backends\utils.py", line 98, in execute
    return super().execute(sql, params)
  File "c:\Users\Cody\Desktop\374 Dev\my_project\my_projectEnv\lib\site-packages\django\db\backends\utils.py", line 66, in execute
    return self._execute_with_wrappers(sql, params, many=False, executor=self._execute)
  File "c:\Users\Cody\Desktop\374 Dev\my_project\my_projectEnv\lib\site-packages\django\db\backends\utils.py", line 75, in _execute_with_wrappers
    return executor(sql, params, many, context)
  File "c:\Users\Cody\Desktop\374 Dev\my_project\my_projectEnv\lib\site-packages\django\db\backends\utils.py", line 84, in _execute
    return self.cursor.execute(sql, params)
  File "c:\Users\Cody\Desktop\374 Dev\my_project\my_projectEnv\lib\site-packages\django\db\utils.py", line 90, in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
  File "c:\Users\Cody\Desktop\374 Dev\my_project\my_projectEnv\lib\site-packages\django\db\backends\utils.py", line 84, in _execute
    return self.cursor.execute(sql, params)

Exception Type: ProgrammingError at /admin/ProjectManager/structure/1/change/
Exception Value: can't adapt type 'Quantity'
CodyMorton commented 2 years ago

Hi, do you think you could take a look and see if you see any reason why the DecimalQuantityField's aren't working for us?

Thank you so much for your help :)

CarliJoy commented 2 years ago

Sorry didn't have time to debug. Could you maybe debug your self:

I assume that this line:


        if isinstance(value, self.ureg.Quantity):
            to_save = value.to(self.base_units)
            return self.to_number_type(to_save.magnitude)
        return value

isn't working for some reason. If you return the value itself, that something is wrong! It should convert to the pure number type (Decimal) in this example.

So the reason is not to delete the super call but to fix this .

CodyMorton commented 2 years ago

That does seem to be working, here is what happens:

Entering that line: image

Exiting the function it does return a Decimal(); image

What get's sent to the clean() function afterwards: image

It finally makes it to my view, and the form begins to save. image

I step into the new_struct.save():

Notice how it exists correct when it enters save(): image

It also goes through the get_db_prep_save() function and it sends in a Decimal: image

This ends up in the prepare_value() function where it turns back into a Quantity: image

Then when it's in as_sql, it has a Quantity field and tries to make sql for it: image

Which eventually results in the final error: image

Do you know why the super().get_db_prep_save() function would make it turn back into a Quantity?

Thank you for your help! This is where I get stuck while debugging.

CodyMorton commented 2 years ago

To summarize: During model.save(), it passes through get_prep_value(), which correctly returns a Decimal(). It eventually sends a Decimal() to SQLInsertCompiler.prepare_value(): image Which sends the Decimal to the field's (DecimalQuantityField) get_db_prep_save(). And that uses get_prep_value() again, but returns a Quantity(). image

CodyMorton commented 2 years ago

Some further debugging that I should have done shows that this actually goes to the QuantityField.to_python() function which returns a Quantity object. It starts here in DecimalQuantityField as a Decimal:

    def to_python(self, value) -> Quantity:
        if isinstance(value, (str, float, int)):
            value = models.DecimalField.to_python(self, value)
        return QuantityField.to_python(self, value)

My understanding is that if something that's NOT a Decimal() is passed in here (a str, float, or int, for example), then it should use the DecimalField.to_python function which is supposed to:

Convert the input value into the expected Python data type, raising
        django.core.exceptions.ValidationError if the data can't be converted.
        Return the converted value. Subclasses should override this.

Then, it converts to a Quantity object using the QuantityField to_python.

    def to_python(self, value) -> Quantity:
        if isinstance(value, self.ureg.Quantity):
            return value

        if value is None:
            return None

        return self.ureg.Quantity(value * getattr(self.ureg, self.base_units))

But shouldn't this convert or keep as a Decimal since it's doing this under get_db_prep_save?

EDIT: This explains that it SHOULDN'T do that: https://docs.djangoproject.com/en/3.2/howto/custom-model-fields/

But I don't understand why it's not working for me then, because the error I'm getting says "can't adapt type 'Quantity'." How do you make sure it's able to adapt a custom field type?

Thanks and sorry for the overflow of messages, I hope this makes sense. Thank you!

CarliJoy commented 2 years ago

Dear Cody,

thanks for the feedback and debugging done. I finally found some time and figured out that the problem just came with Django 3.2 -> they changed something that is incompatible with the current implementation. I still couldn't really figure out, what exactly was changed, even so I invested quite some time in it -> sorry. Currently I don't see an easy way that would fix this issues without braking Django pint for older Django versions.

Please consider using Django 3.1 for the time being.

CarliJoy commented 2 years ago

Ok found a fix. See MR linked. Will merge soon and release a new version.

CodyMorton commented 2 years ago

Wow awesome!! I can't believe I didn't test that at first. I really appreciate your help! Thank you! Looking forward to the release.