Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

set attribute to None does not work (at least for fields with default values) #734

Closed
DavidBord opened this issue Aug 13, 2014 · 13 comments · Fixed by #767
Closed

set attribute to None does not work (at least for fields with default values) #734

DavidBord opened this issue Aug 13, 2014 · 13 comments · Fixed by #767
Labels

Comments

@DavidBord
Copy link
Contributor

    class User(Document):
        name = StringField()
        height = IntField(default=184)
    User.objects.delete()
    u = User(name='user')
    u.save()
    u_from_db = User.objects.get(name='user')
    u_from_db.height = None
    u_from_db.save()
    self.assertEquals(u_from_db.height, None)
@rozza
Copy link
Contributor

rozza commented Aug 14, 2014

This has long been a bone of contention - I think you need to delete the value to remove it but that may have changed.

Essentially, default means the value if its missing and not the value when first set - which is the issue.

@thedrow
Copy link
Contributor

thedrow commented Aug 14, 2014

This is definately a bug.

@rozza
Copy link
Contributor

rozza commented Aug 14, 2014

Copying comment from #735

Default should be used on the creation of totally new objects only and not from the database. This would be a breaking change and might get some people relying on the current behaviour (eg to mimic last_updated with a default callable of now).

@DavidBord
Copy link
Contributor Author

When I set the value to None, I don't want to remove it - I want to set it to None but it does not happen

DavidBord added a commit to DavidBord/mongoengine that referenced this issue Aug 17, 2014
@rozza
Copy link
Contributor

rozza commented Aug 20, 2014

Here's the crux None is not a valid String, Number, EmbeddedDocument etc..

>>> type(None) == type('string')

False

If you set a StringField to None its string representation is "None" which is not correct.
None is a valid BSON type and can be stored in MongoDB - but is that the type you want to store or would it be better not to store a value at all? What happens if its required?

@thedrow
Copy link
Contributor

thedrow commented Aug 21, 2014

If it's required it should be null.

@DavidBord
Copy link
Contributor Author

  1. I wouldn't expect that set to an attr to None would unset it. I think it is even better for it to fail on validation
  2. Why not make None explicitly a valid value just like in Django?

@rozza
Copy link
Contributor

rozza commented Aug 21, 2014

In response:

  1. Ok so validation would fail as the types wouldn't match - we could stop people setting the value to None
  2. We could be we still have to clarify what happens in various scenarios

So a couple more questions to help spec this out:

  1. If we set a value to None - should we store it as null in the database or not store it at all?
  2. Does required=True v required=False change behaviour?
  3. What should we do with new models who by definition have default values as null set them to None or simply not set them?

@DavidBord
Copy link
Contributor Author

  1. Store it as null
  2. No
  3. Set them to None

DavidBord added a commit to DavidBord/mongoengine that referenced this issue Sep 18, 2014
@mitar
Copy link
Contributor

mitar commented Oct 27, 2014

It seems this could address #381 as well.

DavidBord added a commit to DavidBord/mongoengine that referenced this issue Nov 7, 2014
DavidBord added a commit to DavidBord/mongoengine that referenced this issue Nov 7, 2014
@mitar
Copy link
Contributor

mitar commented Nov 10, 2014

Great! When will this be in a released version?

@jimmyshen
Copy link
Contributor

Hi,

I was playing with the new 'null' behavior, which I would like to use in a project I'm working on. While messing with it on master, I noticed some inconsistency in the behavior with regard to DateTimeField and ComplexDateTimeField.

Here's a demonstration:

def test_null_datetime_fields():
    class TestDoc(Document):
        dt_field = DateTimeField(null=True)
        cdt_field = ComplexDateTimeField(null=True)

    doc = TestDoc()
    doc.save()

    assert doc.dt_field is None
    assert doc.cdt_field is None

The cause seems to be related to some logic specific to ComplexDateTimeField that falls back to datetime.now on retrieval from db. I'm not sure what the motivation was for the different behavior between DateTimeField and ComplexDateTimeField -- it was introduced a long time ago and there's even a test to ensure it doesn't regress:

0187a0e1

I made a PR with a teeny patch that (I think) remedies the behavior:

#864

@RussellLuo
Copy link
Contributor

@DavidBord, has this issue been fixed? I use MongoEngine v0.10.0, but the nullable field will still be deleted if I set its value to None:

class User(Document):
    name = StringField(null=True)

user = User(name='user')
# the name field is stored in MongoDB
user.save()

user.name = None
# the name field is deleted from MongoDB
user.save()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants