Opened 12 years ago
Closed 12 years ago
#19326 closed New feature (fixed)
Add a first method to the QuerySet
Reported by: | Wim Feijen | Owned by: | nobody |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | first |
Cc: | mike@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Many times, get() is being used inside a try except Model.DoesNotExist clause. In order to save us lines of code and in order to prevent the Exception becoming common, a discussion followed which can be found here:
According to our BDFL:
After thinking a bit, I'm +1 on the idea, and I think
Queryset.first()
is the right name (which implies we should probably
have a last()
for completion).
This mirrors the name used by a few other ORM/collection APIs that I
think have really nice APIS:
- SQLalchemy (http://docs.sqlalchemy.org/en/rel_0_7/orm/query.html#sqlalchemy.orm.query.Query.first)
- Storm (http://people.canonical.com/~therve/storm/storm.store.ResultSet.html#first)
- Backbone (http://backbonejs.org/#Collection-Underscore-Methods /
http://documentcloud.github.com/underscore/#first)
If someone produces a patch I'll review it and commit it.
Jacob
Attachments (2)
Change History (12)
by , 12 years ago
Attachment: | 19326.diff added |
---|
comment:1 by , 12 years ago
Something like this should work:
try: return self.filter(*args, **kwargs)[0] except IndexError: return None
I am not sure if the IndexError
is correct error type...
If .last() is needed then that is going to be a bit more problematic as you will need to revert all of the ordering clauses of the query.
by , 12 years ago
Attachment: | 19326_2.diff added |
---|
My second take, using filter and IndexError as suggested
comment:2 by , 12 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:4 by , 12 years ago
Cc: | added |
---|
comment:5 by , 12 years ago
Some polish done, available from https://github.com/akaariai/django/compare/ticket_19326
Biggest changes are addition of .last() method for symmetry, and automatic ordering by PK if there is no ordering defined otherwise. In addition some docs changes and move of the tests to new test module first_and_last.
I will still ask at django-developers if the API is the wanted one, otherwise this should be 100% ready for commit.
comment:6 by , 12 years ago
Triage Stage: | Ready for checkin → Design decision needed |
---|
I'm setting this to DDN. If these methods were added, then the API would be confusing. .latest() vs .last() and .earliest() vs .first() will be confusing.
I still think it would be nice to have some way to get None instead of DoesNotExist if the None is expected, not exceptional value. However adding anything that is confusing is a bad idea.
Another API idea: .default(None).get(...), .default(someobj).latest() or .default(None).order_by('somecol')[0] could all make the ORM return None or any other given value instead of DoesNotExist. Does this sound sensible? Naming is of course tentative, for example .default_on_empty() could be better if somewhat long...
comment:7 by , 12 years ago
Triage Stage: | Design decision needed → Accepted |
---|
I haven't seen a proposal addressing the first/last vs earliest/latest confusion that isn't even *more* confusing, so I'm marking this accepted. Let's just do first/last + earliest/latest and live with the slight redundancy.
comment:8 by , 12 years ago
I made a pull request implementing these features here: https://github.com/django/django/pull/1056 . Let me know if there's anything else that I need to change.
comment:9 by , 12 years ago
How about adding a test that first()
and last()
work as expected on empty querysets? Other than that the patch looks good to me.
comment:10 by , 12 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Patch for ticket 19326: added first. Unfortunately, my implementation fails