> Or how about using something similar to QuearyDict's get(key, > default)? That way the fallback is not restricted to `None`. So > something like this:
> user = User.objects.get(pk=user_id, default=None)
> Of course, I think that would probably be significantly more work than > your little wrapper function.
# The string constant used to separate query parts LOOKUP_SEPARATOR = '__' +# for get(default) +NOT_SET='WHATEVER'
# The list of valid query types QUERY_TERMS = ( @@ -202,7 +204,8 @@ cursor.execute("SELECT COUNT(*)" + sql, params) return cursor.fetchone()[0]
- def get(self, *args, **kwargs): + + def get(self, default=NOT_SET, *args, **kwargs): "Performs the SELECT and returns a single object matching the given keyword arguments." clone = self.filter(*args, **kwargs) # clean up SQL by removing unneeded ORDER BY @@ -210,6 +213,8 @@ clone._order_by = () obj_list = list(clone) if len(obj_list) < 1: + if default != NOT_SET: + return default raise self.model.DoesNotExist, "%s matching query does not exist." % self.model._meta.object_name assert len(obj_list) == 1, "get() returned more than one %s -- it returned %s! Lookup parameters were %s" % (self.model._meta.object_name, len(obj_list), kwargs) return obj_list[0]
Nice job Honza. I was about to suggest just about the same, but hesitated because of the *args and **kwargs. I just wasn't sure why that made me hesitate.
On 12/18/06, SmileyChris <smileych...@gmail.com> wrote:
> On Dec 19, 11:08 am, "Waylan Limberg" <way...@gmail.com> wrote: > > something like this:
> > user = User.objects.get(pk=user_id, default=None)
> Except this would break (or at least limit the functionality of) > objects which use "default" as a field name.
Now I know why. Good call SmileyChris. Unfortunately I have no ideas for a workaround.
SmileyChris wrote: > On Dec 19, 11:08 am, "Waylan Limberg" <way...@gmail.com> wrote: > > something like this:
> > user = User.objects.get(pk=user_id, default=None)
> Except this would break (or at least limit the functionality of) > objects which use "default" as a field name.
Of course, this didn't stop get_or_create(), which uses "defaults" as a parameter. If you had an object with an attribute named "default" then you could still do something like:
But get_or_create() was new functionality and this would be changing existing functionality. If that is unacceptable, then maybe a get_or_default() that optionally takes the "default" parameter, using None if no "default" specified.
So... MyModel.objects.get_or_default(pk=1) would be equivalent to MyModel.objects.get_or_default(pk=1, default=None)
There should also be an update_or_create() function. I often find myself get_or_creating, then updating the object's dict if it wasnt just created, then saving. This can easily be expressed more clearly in one line with this function. j
On 12/20/06, Gary Wilson <gary.wil...@gmail.com> wrote:
> SmileyChris wrote: > > On Dec 19, 11:08 am, "Waylan Limberg" <way...@gmail.com> wrote: > > > something like this:
> > > user = User.objects.get(pk=user_id, default=None)
> > Except this would break (or at least limit the functionality of) > > objects which use "default" as a field name.
> Of course, this didn't stop get_or_create(), which uses "defaults" as a > parameter. If you had an object with an attribute named "default" then > you could still do something like:
> But get_or_create() was new functionality and this would be changing > existing functionality. If that is unacceptable, then maybe a > get_or_default() that optionally takes the "default" parameter, using > None if no "default" specified.
> So... > MyModel.objects.get_or_default(pk=1) > would be equivalent to > MyModel.objects.get_or_default(pk=1, default=None)
def update_or_create( --same arguments as get_or_create()-- ): object, created = Class.objects.get_or_create( --args above-- ) if not created: object.__dict__.update(defaults) object.save() return object
Well, Class is self or this or whatever it is in python/djangomanagers code. I'm also not sure how to do the crazy **kwargs stuff. The rest is clear: if you didn't create it, just update it, save it, and return. j
On 12/20/06, Joseph Perla <josephpe...@gmail.com> wrote:
> There should also be an update_or_create() function. I often find myself > get_or_creating, then updating the object's dict if it wasnt just created, > then saving. This can easily be expressed more clearly in one line with > this function. > j
> On 12/20/06, Gary Wilson <gary.wil...@gmail.com> wrote:
> > SmileyChris wrote: > > > On Dec 19, 11:08 am, "Waylan Limberg" <way...@gmail.com> wrote: > > > > something like this:
> > > > user = User.objects.get (pk=user_id, default=None)
> > > Except this would break (or at least limit the functionality of) > > > objects which use "default" as a field name.
> > Of course, this didn't stop get_or_create(), which uses "defaults" as a > > parameter. If you had an object with an attribute named "default" then > > you could still do something like:
> > But get_or_create() was new functionality and this would be changing > > existing functionality. If that is unacceptable, then maybe a > > get_or_default() that optionally takes the "default" parameter, using > > None if no "default" specified.
> > So... > > MyModel.objects.get_or_default (pk=1) > > would be equivalent to > > MyModel.objects.get_or_default(pk=1, default=None)
Joseph Perla wrote: > There should also be an update_or_create() function. I often find myself > get_or_creating, then updating the object's dict if it wasnt just created, > then saving. This can easily be expressed more clearly in one line with > this function.
This gave me the idea of an update() method for model instances. I have created a ticket for this with code, documentation, and tests.
Joseph Perla wrote: > Also, here's the pseudo-ish code for it:
> def update_or_create( --same arguments as get_or_create()-- ): > object, created = Class.objects.get_or_create( --args above-- ) > if not created: > object.__dict__.update(defaults) > object.save() > return object
> Well, Class is self or this or whatever it is in python/djangomanagers > code. I'm also not sure how to do the crazy **kwargs stuff. The rest is > clear: if you didn't create it, just update it, save it, and return.
I have created a ticket for this too and have attached some code:
Hm - I get a whole bunch of failures when I apply the patch. Looking closer, you say it depends on an ``update()`` patch in #3181, but I don't see any such patch attached to #3181.
Can you update #3182 to include this other patch (wherever it lives), and close the other ticket as a dup of #3182? Once that's done I'll go ahead and check this in.
Jacob Kaplan-Moss wrote: > Hm - I get a whole bunch of failures when I apply the patch. Looking closer, > you say it depends on an ``update()`` patch in #3181, but I don't see any such > patch attached to #3181.
Sorry about that, it should be #3180.
> Can you update #3182 to include this other patch (wherever it lives), and > close the other ticket as a dup of #3182? Once that's done I'll go ahead and > check this in.
Well, the two tickets are different functionality. I could create a single patch for both, the only thing was that in the patch for #3180 I added an "Updating objects" to the db api documentation, but then realized that it should probably be integrated into the "Saving changes to objects" section. I haven't spent the time to do that yet.
> Well, the two tickets are different functionality. I could create a > single patch for both, the only thing was that in the patch for #3180 I > added an "Updating objects" to the db api documentation, but then > realized that it should probably be integrated into the "Saving changes > to objects" section. I haven't spent the time to do that yet.
Yeah, they're different... but I think they make the most sense if they go in as a unit. I'd prefer a single patch (and tests, docs, etc.) if you're up to it.
Jacob Kaplan-Moss wrote: > On 12/24/06 11:15 PM, Gary Wilson wrote: > > Well, the two tickets are different functionality. I could create a > > single patch for both, the only thing was that in the patch for #3180 I > > added an "Updating objects" to the db api documentation, but then > > realized that it should probably be integrated into the "Saving changes > > to objects" section. I haven't spent the time to do that yet.
> Yeah, they're different... but I think they make the most sense if they go in > as a unit. I'd prefer a single patch (and tests, docs, etc.) if you're up to it.
Sure, I'll work on the documentation fixes and let you know when done.
Gary Wilson wrote: > Jacob Kaplan-Moss wrote: > > On 12/24/06 11:15 PM, Gary Wilson wrote: > > > Well, the two tickets are different functionality. I could create a > > > single patch for both, the only thing was that in the patch for #3180 I > > > added an "Updating objects" to the db api documentation, but then > > > realized that it should probably be integrated into the "Saving changes > > > to objects" section. I haven't spent the time to do that yet.
> > Yeah, they're different... but I think they make the most sense if they go in > > as a unit. I'd prefer a single patch (and tests, docs, etc.) if you're up to it.
> Sure, I'll work on the documentation fixes and let you know when done.
Ok. I reworked the documentation, rolled both patches into one, and attached it to #3182.
Ok, after getting sidetracked by update() and update_or_create(), back to the topic of the OP. Here are some options:
1) Add a get_or_none() method that returns None if object does not exist.
2) Add a get_or_default() method that accepts a "default" keyword argument, the value of which gets returned if object does not exist. If a default is not specified then None gets returned by default.
3) Modify the current get() method to accept a "default" keyword argument as in option 2), except that instead of returning None if "default" is not specified, a DoesNotExist exception is raised like normal. To return None, you would have to explicitly say "default=None".
Note that this would also be backwards incompatible in that models with a field named "default" would have to use "default__exact=..." instead of "default=..." as with get_or_create(). Looking further, it looks like functions that use get() might also have to change slightly, i.e. get_or_create().
> Ok, after getting sidetracked by update() and update_or_create(), back > to the topic of the OP. Here are some options:
> 1) Add a get_or_none() method that returns None if object does not > exist.
> 2) Add a get_or_default() method that accepts a "default" keyword > argument, the value of which gets returned if object does not exist. > If a default is not specified then None gets returned by default.
> 3) Modify the current get() method to accept a "default" keyword > argument as in option 2), except that instead of returning None if > "default" is not specified, a DoesNotExist exception is raised like > normal. To return None, you would have to explicitly say > "default=None".
> Note that this would also be backwards incompatible in that models with > a field named "default" would have to use "default__exact=..." instead > of "default=..." as with get_or_create(). Looking further, it looks > like functions that use get() might also have to change slightly, i.e. > get_or_create().
> Comments/votes/suggestions?
I like 3, but considering the potential complications 2 may make more sense. Either way, I don't see the value in 1 as in either 2 or 3, the default can still be set to None.
> > Documentation and tests attached too now.Hm - I get a whole bunch of failures when I apply the patch. Looking closer, > you say it depends on an ``update()`` patch in #3181, but I don't see any such > patch attached to #3181.
> Can youupdate#3182 to include this other patch (wherever it lives), and > close the other ticket as a dup of #3182? Once that's done I'll go ahead and > check this in.
On Sat, 2007-03-10 at 15:54 +0000, whiteinge wrote: > If someone has a moment I would appreciate hearing a status update for > this ticket--it reports ready for checkin, will it still be included > in Django?
> I tested the patch and it works as advertised, also the docs are well > written. This is functionality I'm looking forward to.
It will probably be applied at some point. It is one of a number of open tickets. Sorry, but there are lots of things we want to work on all at the same time.
|
