Implement an opposite method for Entry.objects.latest

[Artem Skoretskiy]

At the moment we have Entry.objects.latest method that efficiently gets the last entry in queryset ​https://docs.djangoproject.com/en/dev/ref/models/querysets/#latest

We need another handy method that would do the same but getting the first element.

I suggest to name it first or something like that.

I have attached a sample patch.

[Aymeric Augustin]

What's the use case for this method?

[Russell Keith-Magee]

Consider the case of an event log. Retrieving the most recent event has an easy shortcut:

>>> Event.objects.latest('timestamp')

but getting the first event doesn't, so you need to do it the long way:

>>> Event.object.order_by('timestamp')[0]

Retrieving the first event by itself isn't an obvious query, but something like "Get me the first event this week" would be.

For me, this is a fairly obvious piece of symmetry in the ORM API; I've wanted it to exist a few times in recent history.

[jimmysong]

added tests, fixed the fact that the model manager didn't have a first() method.

[jimmysong]

First open source contribution here, FYI.

I've added a test for the first method and added a first method to the model manager. The tests pass.

There is one thing in the patch that's inconsistent and that's the get_latest_by from the _meta. It seems too hackish to leave that as the default for the first as well. I would like some direction as to how to how to solve this dilemma. Creating a corresponding get_first_by seems really odd as Entry.objects.first() would use a completely different criteria as Entry.objects.latest(). Creating a more general thing to apply to first or latest is a naming nightmare and get_first_or_latest_by is neither backwards-compatible nor easy to remember.

Can someone give me some direction on this?

[Adrien Lemaire]

I read your code. Tests need to be renamed, it's not LatestTests anymore but FirstOrLatestTests.
Same goes for the model.Options.get_latest_by. Don't make any sense to ask for a get_latest_by parameter when performing and first() action.

Two solutions:

• rename get_latest_by in get_first_or_latest_by . Backward incompatible, but that options touch a very tiny bit of code, and this would be the cleanest solution.
• add a new get_first_by field in Options that will behave like get_latest_by.

I'll add a new patch with the 2nd solution, and will change it if the core developers agree with the first one.

[Adrien Lemaire]

Added new get_first_by Objects field. More tests. Doc updated

[Adrien Lemaire]

Done. I also added more tests to insure the correct error is raised if no Meta.get_first_by or Meta.get_latest_by is defined

[Henrique C. Alves]

For the record: a method named first is misleading. The antonym for "latest" is "earliest". first sounds too much like you're getting the first record (by PK order) instead of by an arbitrary field.

Also, having 2 different attributes on Meta is odd. I don't see why you can't just keep get_latest_by and get along without breaking everybody's code. It will be obvious earliest is reciprocal.

[Adrien Lemaire]

You're right for first, I just went along with jimmysong implementation without thinking about that. Will change that now

For the attributes, what is odd is to add a Meta.get_latest_by in your model when you want to call a earliest(), don't you think ? It's also more flexible to have 2 separate attributes, therefore I finally think it's the best approach. And it doesn't break everybody's code.

[Adrien Lemaire]

new diff with first renamed into earliest

[Adrien Lemaire]

Done.

[Henrique C. Alves]

Replying to Fandekasp:

For the attributes, what is odd is to add a Meta.get_latest_by in your model when you want to call a earliest(), don't you think ?

I don't think so. What I would consider odd, though, is having a second, analagous property, get_earliest_by, when you can just derive one from the other and stay DRY. As long as ealiest() is explained in the docs as doing the opposite of latest(), there's no more room for confusion.

It's also more flexible to have 2 separate attributes, therefore I finally think it's the best approach.

I'm afraid this is a case were developers expect behaviour to be more obvious than flexible. Allowing latest() and earliest() to return different results is guaranteed to be a source of stupid bugs, while having no valid use case.

[Adrien Lemaire]

I see your point. Let's wait for a core developer comment

[Julien Phalip]

Thanks for the patch. Like hcarvalhoalves, I think it would be non-DRY and potentially confusing to allow for two attributes. get_latest_by is sufficient to indicate which field should be used for ordering, so there is no need for an extra attribute. Also, renaming get_latest_by would be unnecessarily backwards-incompatible. So, all that's needed is a new earliest() method that uses the inverse order.

[Adrien Lemaire]

get_earliest_by attr related code reverted

[Adrien Lemaire]

done

[Julien Phalip]

The patch looks pretty good. There's still one occurrence of 'get_earliest_by' in the tests. Could you remove it? When applying the patch I've got some warnings that there were trailing whitespaces. If you could remove those as well, that would be great. Thanks!

[Adrien Lemaire]

Sorry for the occurence of 'get_earliest_by' in the tests. I'm pretty sure I fixed it, so I messed up somewhere when creating the diff. Fixed now.

@julien, what are you using to get those trailing whitespaces warnings ?
I'm using flake8 usually, but I gave up on the django project:
$ flake8 .|wc
20744 148974 1795831
$ flake8 .|grep "trailing whitespace"|wc
407 1628 28496

There are 407 trailing whitespace around the repo ... Anyways, I think I fixed the ones in the files I've edited.

Cheers

[Julien Phalip]

I'm getting a failure when running the tests:

(djangocore-trunk)➜  tests  python runtests.py --settings=test_sqlite get_earliest_or_latest
Creating test database for alias 'default'...
Creating test database for alias 'other'...
.F.
======================================================================
FAIL: test_latest (modeltests.get_earliest_or_latest.tests.EarliestOrLatestTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/julien/Documents/Development/eclipse/workspace/DjangoCore/hg-trunk/tests/modeltests/get_earliest_or_latest/tests.py", line 65, in test_latest
    self.assertRaises(Article.DoesNotExist, Article.objects.latest)
AssertionError: _earliest_or_latest() requires either a field_name parameter or 'get_latest_by' in the model

----------------------------------------------------------------------
Ran 3 tests in 0.010s

FAILED (failures=1)
Destroying test database for alias 'default'...
Destroying test database for alias 'other'...

Also, shouldn't the folder modeltests/get_latest/ be removed?

Re the warnings: I just get those when running 'git apply'. I didn't get them with the latest patch though, so it's fine on that front. Thanks!

[Adrien Lemaire]

Fixed.

• modeltests/get_latest has to be removed yes, forgot to add it in the patch.
• Tests were broken because one will set Article._meta.get_latest_by = None, and the next test will expect it to be something. fixed by adding a tearDown that adds back the get_latest_by to the model if it's None.

[paper82]

Hello, new contributor here - I'd like to ask about the current state of this ticket/patch; if there is something to be done, I'd like to take a shot at it.

[Nick Sandford]

Cleaned up patch a bit. It looks good, tests pass. Pull request here: ​https://github.com/django/django/pull/643.

[Anssi Kääriäinen <akaariai@…>]

In fe54377dae1357a7f102d72614a13f0ef8b2dbdf:

Fixed #17813 -- Added a .earliest() method to QuerySet

Thanks a lot to everybody participating in developing this feature.
The patch was developed by multiple people, at least Trac aliases
tonnzor, jimmysong, Fandekasp and slurms.

Stylistic changes added by committer.

[Anssi Kääriäinen <akaariai@…>]

In f96c86b02943009d4c2e01d8e4457db040723a25:

Added missing versionadded 1.6 to docs of earliest()

Refs #17813

[Anssi Kääriäinen <akaariai@…>]

In 7aa538357c8d94df3d5811706bf6dfe5d21421ca:

Cleaned up testing models.py added for earliest()

The main cleanup was removal of non-necessary unicode method. The
tests didn't break on py3 as the string representation was never used
in the tests.

Refs #17813. Thanks to Simon Charette for spotting this issue.