`refresh_from_db()` should return the model instance instead of None

[Tom Forbes]

It is mildly annoying that refresh_from_db returns None instead of self. In some scenarios (especially involving tests) it would be quite nice to be able to do:

assert model.refresh_from_db().status == SomeEnum.SomeStatus

With lists of models it's also nice to be able to do:

assert all(model.refresh_from_db().status == Something for model in list_of_models)

Currently because the refresh_from_db returns None you cannot do this.

[Tim Graham]

I'm not sure. I thought it might have been discussed before, but if so, I can't find it. Model.save() doesn't return self either which seems like a similar situation. Can you think of anywhere else in Django (or Python) where this pattern is used?

Considerations:

• Anyone overriding the method may have to add a return value to adapt their code for the new convention.
• For new users, it may make it unclear whether or not the obj on which the refresh_from_db() is called is updated, or if it's required to do obj = obj.refresh_from_db().

[Jon Dufresne]

I've noticed that Python tends to return None for functions that mutate the object in place and return a new object for non-mutating functions. Just one example:

>>> type([1].sort())
<class 'NoneType'>
>>> type(sorted([1]))
<class 'list'>

It may not be a hard rule, but could be considered as a guideline.

[Carl Meyer]

It's definitely a strong Python design guideline that mutating-in-place methods do not also return self. Personally I wouldn't mind if refresh_from_db only returned an updated object, and didn't mutate the existing instance, but that ship has sailed. I don't think it should do both.

[Aymeric Augustin]

In fact the reason why refresh_from_db exists is not to return an updated object, but instead to refresh it in place so that other pieces of code that have kept a reference to this instance see the updated data.

To return a copy, Thing.objects.get(pk=thing.pk) is hardly longer than thing.refresh_from_db() and (in my opinion) should be preferred because it's explicit.

A shorter and more idiomatic way to write:

assert all(model.refresh_from_db().status == Something for model in queryset)

is

assert all(model.status == Something for model in queryset.all())

(The .all() creates a new queryset that forces a new database query.)

If refresh_from_db() returns the updated instance, it will be used in situations when it isn't necessary and where the regular, safer APIs that don't involve in place mutations are a better choice.