Opened 9 years ago

Closed 6 years ago

Last modified 3 years ago

#5741 closed Uncategorized (wontfix)

make queryset get(), latest(), earliest(), take a default kwarg

Reported by: Dan Watson Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords: queryset get latest earliest default
Cc: hv@…, Ben Finney Triage Stage: Design decision needed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


Just to follow up on so it doesn't get forgotten. I know this is supposed to wait for the queryset refactoring to be completed, but I figured it couldn't hurt to have a working patch and doc changes now. And as it stands now, the patch would look virtually identical against the queryset-refactor branch.

Attachments (3)

get_default.diff (1.9 KB) - added by Dan Watson 9 years ago.
5741.diff (2.3 KB) - added by Dan Watson 8 years ago.
updated patch against r8194
get_with_default.diff (2.6 KB) - added by Malcolm Tredinnick 8 years ago.
Updated patch (just the docs bit changed)

Download all attachments as: .zip

Change History (14)

Changed 9 years ago by Dan Watson

Attachment: get_default.diff added

comment:1 Changed 9 years ago by Thomas Güttler <hv@…>

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

Yes, a default argument (like a dictionary) would be good.

Since 95% I use the primary key if I use get(), I suggest this
dict like API:

foo=Foo.objects.get(myid, default)

this should behave like

    foo=Foo.objects.get(**{ myid})
except Foo.DoesNotExist:

comment:2 Changed 9 years ago by Thomas Güttler <hv@…>

Cc: hv@… added

comment:3 in reply to:  1 Changed 9 years ago by Will Hardy

Or maybe something like:

  foo = Foo.objects.default(None).get(myid)

This would be backwards compatible and would also avoid any name-conflict woes. Of course, I don't like that it's not so readable anymore, but it might be useful.

Another approach is to have get return None instead of an exception:

  foo = Foo.objects.get(myid) or default_object

But I doubt that the whole DoesNotExist exception thing would be thrown away at this stage. Maybe a settings variable can be used to "turn on" this functionality, to keep it backways compatible and let the user choose whether or not they want exceptions or return values. This road is getting a little messy.

A third option is:

  foo = Foo.objects.get_or_none(myid) or default_object

...but the method needs a better name, because it's really starting to lose it's readability, but I like it better than the first one.

comment:4 Changed 9 years ago by Dan Watson

The patch adds a "default" kwarg, that returns the value of "default" if it's supplied, and raises the DoesNotExist exception otherwise, like before. So you can do either of the following:

  foo = Foo.objects.get( pk=myid )
except Foo.DoesNotExist, ex:
  foo = None


foo = Foo.objects.get( pk=myid, default=None )

I think making get() use the first argument as a primary key lookup is unnecessary with the "pk" shortcut. Also, I'd imagine it would add a lot of ugly special-casing, since get() is really a convenience for filter(), whose positional args (if provided) are expected to be Q objects.

comment:5 Changed 9 years ago by Jacob

Triage Stage: UnreviewedAccepted

Changed 8 years ago by Dan Watson

Attachment: 5741.diff added

updated patch against r8194

comment:6 Changed 8 years ago by Malcolm Tredinnick

Triage Stage: AcceptedDesign decision needed

Dan's going to want to kill me (sorry, Dan!), but we can't commit this as it is. It's backwards incompatible. If somebody has a qs.get(foo=1, default=2) call now, that code will break with this change.

So if this is really, really considered necessary (and the bar just got higher because of this), it need to be a method with a new name. What I don't want to see are 23 comments attached to this ticket suggesting everybody's favorite name for the new pony. Somebody (hi, Dan!) should come up with a good name or two and gather some consensus on django-dev, which should probably include some kind of glimmer of interest from somebody with commit privileges. I'm kind of somewhere around +0/-0 on this. I use the pattern a fair bit myself, but it doesn't worry me too much (it's a three line utility function if I don't want to catch exceptions too often). Since it's introducing a whole new method, it's going to need some support. I know I'm not really helping there, but maybe I'm undervaluing the ticket's utility so I'm not going to wontfix it immediately.

As some consolation, here's an updated patch that at least applies against trunk. So it can be the starting point for modification.

Changed 8 years ago by Malcolm Tredinnick

Attachment: get_with_default.diff added

Updated patch (just the docs bit changed)

comment:7 Changed 8 years ago by Dan Watson

I'm fine with relegating it to utility-function land. It may have been nice to mirror dict.get(), but it's by no means difficult to catch the exception, create a utility function, or even have your own manager get() method. Or perhaps it would be better suited as a get_object method in django.shortcuts. In any case, I don't feel strongly enough about it to carry the torch, so feel free to wontfix it or defer it to 2.0, when we can break stuff with reckless abandon!

comment:8 Changed 7 years ago by kevinh

Can we put this in the django.shortcuts?


def get_object_or_none(klass, *args, **kwargs):
    Uses get() to return an object, or return None if the
    does not exist.
    klass may be a Model, Manager, or QuerySet object. All other passed
    arguments and keyword arguments are used in the get() query.
    Note: Like with get(), an MultipleObjectsReturned will be raised if more than one
    object is found.
    queryset = _get_queryset(klass)
        return queryset.get(*args, **kwargs)
    except queryset.model.DoesNotExist:
        return None

comment:9 Changed 6 years ago by Malcolm Tredinnick

Resolution: wontfix
Status: newclosed

Due to backwards incompatibility changes, this can't be done. Sorry. :-(

Writing one's own shortcut is easy enough here.

comment:10 Changed 6 years ago by anonymous

Severity: Normal
Type: Uncategorized

This is the sort of thing that would be useful if you ever do a big API break (django 2.0) for instance. - Maybe it would be worth collecting all these somewhere like that, so a future version after an API break could benefit?

comment:11 Changed 3 years ago by Ben Finney

Cc: Ben Finney added
Easy pickings: unset
Keywords: latest earliest added
Summary: make queryset get() take a default kwargmake queryset get(), latest(), earliest(), take a default kwarg
UI/UX: unset

We are looking for the same behaviour, but from the ‘QuerySet.latest’ and friends.

Quite a lot of our code looks like this:

foo_set = Foo.filter(spam="Lorem", beans="Ipsum")
    foo = foo_set.latest()
except Foo.DoesNotExist:
    foo = None

This would be cleaner (and less prone to bugs) written as:

foo_set = Foo.filter(spam="Lorem", beans="Ipsum")
foo = foo_set.latest(default=None)

This is a generally-useful feature, and doesn't belong tacked onto every model manager; it should be in the QuerySet API so every model gets it.

Note: See TracTickets for help on using tickets.
Back to Top