coddingtonbear / django-measurement

Easily store, retrieve, and convert measurements of weight, volume, distance, area and more.
MIT License
145 stars 32 forks source link

Modelserializer in django rest_framework serializes MeasurementField as FloatField #82

Closed SOMGER closed 6 years ago

SOMGER commented 6 years ago

hi,

I have the following field in my model: MeasurementField(measurement=Weight, unit_choices=(('kg','kg'),('lb','lb'),))

which serializes as a FloatField() in django rest framework using serializers.ModelSerializer. This FloatField() then does not accept a Mass() object so I land on a TypeError.

Anyone with an idea how to rewrite the serializers _torepresantation and _to_internalvalue to handle this error? I tried this which works for the representation part:

 def to_representation(self, obj):
        return {
            'weight': obj.weight.value,
            'unit': obj.weight.unit}

But I cannot get the to_internal_value to work. Besides, there are other fields in the model which I have to rewrite if I use this approach. Thanks in advance for any suggestion!

Could it be an enhancement to include a serializerField in the MeasurementField definition?

codingjoe commented 6 years ago

I didn't use DRF with this package much, but I think a college did: @anneFly do you have any experience with measurements in DRF?

SOMGER commented 6 years ago

I looked into this further and I think the problem is that there is no MultiValueField among the serializers.Fields in DRF, like there is in forms.Fields, so DRF cannot handle what is in your forms.py line 99-110. Maybe this is a problem more for DRF then for your package. At least, I couldn't find anything that would correspond to forms.MultiValueField in DRF. I ended up defining a custom field like so:

class WeightField(serializers.Field):

    def to_representation(self, obj):
        ret = {"weight": obj.value,
                "unit": obj.unit}
        return ret

    def to_internal_value(self, data):
        unit = data[-2:]
        value = data[:-2]
        if unit == 'kg':
            return Weight(kg=value)
        else:
            if unit == 'lb':
                value_mass = Weight(lb=value)
                return value_mass

I still need to add validation that no other units are entered in the form field, but it works. Only found this solution yesterday.

codingjoe commented 6 years ago

how about something like this?

>>> data = "1gram"
>>> units = Weight.UNITS.keys() | Weight.ALIAS.keys()
>>> pattern = re.compile(r'^(?P<value>\d+)(?P<unit>(%s))$' % '|'.join(units))
>>> match = pattern.match(data)
>>> if match is None:
...     raise ValueError("%s is not a valid %s" % (data, Weight.__name__))
...
>>> kwargs = {match.group('unit'): match.group('value')}
>>> Weight(**kwargs)
Mass(g=1.0)
SOMGER commented 6 years ago

Thank you! This works like charm! I only added a \s to the value part of the regex so that inputs with whitespace between value and unit won't return an error. I think this is a good base for a general MeasurmentSerializerField to_internal_value definition. Thanks again!

codingjoe commented 6 years ago

We could also consider to add this to python-measurement to allow string to be passed to __init__. Similar to how Decimals do it.

codingjoe commented 6 years ago

If that be interesting, feel free to open a PR there.