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

Regression between 0.8.1 and 0.8.2 #381

Closed
mitar opened this issue Jun 23, 2013 · 12 comments
Closed

Regression between 0.8.1 and 0.8.2 #381

mitar opened this issue Jun 23, 2013 · 12 comments
Milestone

Comments

@mitar
Copy link
Contributor

mitar commented Jun 23, 2013

In my tests I found a regression between 0.8.1 and 0.8.2. I have such model:

class BlankableEmbedded(InheritableEmbeddedDocument):
    name = mongoengine.StringField(required=True, default='A blank name')
    description = mongoengine.StringField()

class BlankableParent(InheritableDocument):
    embedded = mongoengine.EmbeddedDocumentField(BlankableEmbedded, required=True, default=lambda: BlankableEmbedded())

And in 0.8.1 if I create a new BlankableParent object it gets a default value for embedded. But if I then set embedded to None and try to save it, I get an error. Which is what I expect.

In 0.8.2 when I set embedded to None it is just reset back to default value. Which is strange. And at least not something which happens in Django.

@rozza
Copy link
Contributor

rozza commented Jun 24, 2013

I believe this is a result of #342, setting to None now resets to the
default if exists. I think you want to use del to delete.

On Sunday, June 23, 2013, Mitar wrote:

In my tests I found a regression between 0.8.1 and 0.8.2. I have such
model:

class BlankableEmbedded(InheritableEmbeddedDocument):
name = mongoengine.StringField(required=True, default='A blank name')
description = mongoengine.StringField()

class BlankableParent(InheritableDocument):
embedded = mongoengine.EmbeddedDocumentField(BlankableEmbedded, required=True, default=lambda: BlankableEmbedded())

And in 0.8.1 if I create a new BlankableParent object it gets a default
value for embedded. But if I then set embedded to None and try to save
it, I get an error. Which is what I expect.

In 0.8.2 when I set embedded to None it is just reset back to default
value. Which is strange. And at least not something which happens in Django.


Reply to this email directly or view it on GitHubhttps://github.com//issues/381
.

@mitar
Copy link
Contributor Author

mitar commented Jun 24, 2013

Ehm. No, I want it to set to None, not delete it. The point is that if I want to allow my code to be able to specify different values and I want to forbid None value, now I have to manually check if it is None and delete the field and before I could just assign the value to the field and leave to MongoEngine checking (and my schema definition) to assure that. I thought that the point of MongoEngine is to get rid of such special cases. That I can say: I want this field to be required and if the value is None or field is missing, complain please.

So, Django does not reset to default when you set to None, does it? Why are we doing it here? This is so counter-intuitive.

@rozza rozza closed this as completed Jun 25, 2013
@rozza
Copy link
Contributor

rozza commented Jun 25, 2013

Sorry, ignore the previous comment: should have been #349.

@rozza rozza reopened this Jun 25, 2013
@rozza
Copy link
Contributor

rozza commented Jun 25, 2013

This was an unintended regression between 0.7 and 0.8 branches.

Possible fix: Define in the fields how they handle None values.

@rozza
Copy link
Contributor

rozza commented Jun 25, 2013

Marking to fix in 0.9 - sorry it wasn't intended for the 0.8 branch to change. But I agree I think it is bad.

@mitar
Copy link
Contributor Author

mitar commented Jun 25, 2013

Yes, one way would be that fields should allow easy way to specify what should be None value: should it be None, should it be nothing (deleted field) or should it be default value. This are three different options. And setting to the default value is something which I have not seen elsewhere. It is a nice feature though in some cases, but a bit "too" magical.

Maybe it would be easier to define MongoEngine specific constant value DEFAULT_VALUE. And if you assign that value to the field, field gets assigned a constant value? If you assign None, it gets assigned None. And if you delattr, it gets deleted. This would be the most Python way in my opinion, without too much magic.

Still, if you want, you could allow fields to override this, but this should be the default in my opinion.

@Seraf
Copy link

Seraf commented Apr 12, 2014

Hello,
I need to use django-tastypie-mongoengine, but with a newer version of Mongoengine than the 0.8.1.
This issue break my package deployment as I can't use django-tastypie-mongoengine and mongoengine in the same requirements file.
Can you please update the status of this issue ?

Thanks !

@mitar
Copy link
Contributor Author

mitar commented Oct 27, 2014

Any progress on this?

@DavidBord
Copy link
Contributor

Does #734 solves your problem? (PR will soon be merged in)

@mitar
Copy link
Contributor Author

mitar commented Oct 27, 2014

Where is the pull request?

@DavidBord
Copy link
Contributor

#767

@MRigal
Copy link
Member

MRigal commented Apr 29, 2015

It should have fixed it. Try with 0.9+

If not, please reopen

@MRigal MRigal closed this as completed Apr 29, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants