mongoengine icon indicating copy to clipboard operation
mongoengine copied to clipboard

ListField(FloatField) does not allow NoneType values

Open Adam-Edry opened this issue 5 years ago • 8 comments

I recently had an issue when converting a field from IntField to FloatField. The default value for the field is None and when we tried to use FloatField we were getting a TypeError: float() argument must be a string or a number, not 'NoneType'.

I've tracked it down to this line of code only catching ValueError exceptions while in IntField it also catches the TypeError.

As a temporary workaround I've created my own class that also catches the TypeError and allows me to use None as a value for floats:

class CustomFloatField(FloatField):
    """Floating point number field. Added TypeError check to allow NoneType values"""
    def to_python(self, value):
        try:
            value = super().to_python(value)
        except TypeError:
            pass
        return value

I'm posting this issue to ask if there is a specific reason behind FloatFields not accepting None values because then my custom field might cause me issues down the road or was it just an oversight?

If there isn't a specific reason I'd be happy to contribute a PR that adds it.

Mongoengine version: 0.19.1 Python version: 3.7.5

Adam-Edry avatar Mar 23 '20 16:03 Adam-Edry

At first sight I don't have anything against accepting None for FloatField but we have some tests that are checking this today and that are passing so I'm not sure how to reproduce your issue.

Anyway I'm happy to receive a PR but make sure you have associated tests

    def test_float_ne_operator(self):
        class TestDocument(Document):
            float_fld = FloatField()

        TestDocument.drop_collection()

        TestDocument(float_fld=None).save()

bagerard avatar Mar 24 '20 20:03 bagerard

I'll try to create a failing test as a first step

Adam-Edry avatar Mar 24 '20 20:03 Adam-Edry

Just got hit by this as well, even when setting null=True in the FloatField. If that is set, I think it should definitely not raise an exception on null, right? For now I've switched to DecimalField which doesn't seem to suffer from the same issue, but of course those are semantically not quite exactly the same thing. Also happy to help write a fix for this if there isn't one in the pipeline yet!

lbel avatar Apr 20 '22 08:04 lbel

For reference, in mongoengine 0.26.0 this bug now also appears on DecimalField – had to implement a workaround in our codebase.

lbel avatar Jan 24 '23 10:01 lbel

The following works fine

from mongoengine import *
connect()


class MPO(Document):
    t = FloatField()
    d = DecimalField()

MPO(t=None, d=None).save()

Can you provide a snippet demonstrating your issue @lbel ?

bagerard avatar Jan 24 '23 12:01 bagerard

Also note that null=True is misleading, it only allows None to be set when there is a default value that exist for a field, if you are not using a default value, null=True has no effect. IMO the whole null=True is useless how it is implemented and is misleading

bagerard avatar Jan 24 '23 12:01 bagerard

Thanks for your reply! So your example works for me as well, where it goes wrong is when the FloatField is inside a ListField:

from mongoengine import *

class C(me.Document):
    f = me.ListField(me.DecimalField())

C(f=[1.0]).validate() # ok
C(f=[1.0, None]).validate() # ValidationError: ValidationError (C:None) (1.Could not convert value to decimal: [<class 'decimal.ConversionSyntax'>]: ['f'])

Setting any combination of null=True or a default on the DecimalField doesn't seem to affect this. But maybe I'm just using it wrong, happy to learn!

lbel avatar Jan 25 '23 08:01 lbel

I've changed the title accordingly. And as I explained above null=True won't help in any way here

bagerard avatar Jan 25 '23 13:01 bagerard