System check methods can yield their items instead of creating lists at every layer

[Chris Jerdonek]

I noticed that many of the internal "check()" methods used by Field subclasses return lists (including the empty list) when they can just yield items (or yield nothing). This is because the concrete check() implementations unpack the return value when constructing a new list. See here for one example of such a check() method: ​https://github.com/django/django/blob/f331eba6d576752dd79c4b37c41d981daa537fe6/django/db/models/fields/__init__.py#L196-L205

The proposed change, then, would be to change methods like the following to yield their items instead of creating and returning a list: ​https://github.com/django/django/blob/f331eba6d576752dd79c4b37c41d981daa537fe6/django/db/models/fields/__init__.py#L243-L254

It looks like there are 23 methods in that file with a name starting with _check....

[Chris Jerdonek]

Actually, it looks like this observation may apply to the entire system check framework. Currently, it looks like lists are constructed at every layer in nested fashion, when it appears that it would work equally well for items to be yielded up the stack.

[Mariusz Felisiak]

Thanks for this proposition, however it's ​documented that check function must return a list of messages. Also, folks can extend checks at any level, see ​examples. I don't think it's worth backward compatibility concerns.

[Chris Jerdonek]

Thanks for this proposition, however it's ​documented that check function must return a list of messages. Also, folks can extend checks at any level, see ​examples. I don't think it's worth backward compatibility concerns.

I was talking also about the internal check methods (e.g. the ones whose names start with an underscore), which are called by the public methods. Here is an example: ​https://github.com/django/django/blob/f331eba6d576752dd79c4b37c41d981daa537fe6/django/db/models/fields/__init__.py#L198-L204
You can see the public method calling several internal methods -- unpacking each of their results to create the new list. Each internal layer adds unnecessary list creations.

[Mariusz Felisiak]

Replying to Chris Jerdonek:

You can see the public method calling several internal methods -- unpacking each of their results to create the new list. Each internal layer adds unnecessary list creations.

True, but I don't think that's a big issue. I'd like to keep internal methods consistent with what we have in docs (we recommend creating internal _check... methods which use this pattern).