[Django] #33014: ProjectState.__init__() can assume its real_apps argument is a set

11 views
Skip to first unread message

Django

unread,
Aug 11, 2021, 1:18:40 PM8/11/21
to django-...@googlegroups.com
#33014: ProjectState.__init__() can assume its real_apps argument is a set
------------------------------------------------+------------------------
Reporter: Chris Jerdonek | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: dev
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
------------------------------------------------+------------------------
[https://github.com/django/django/pull/14760 PR #14760] made all calls to
`ProjectState.__init__()` pass `real_apps` as a set. In
[https://github.com/django/django/blob/54a30a7a00fea6c5e3702282ade6e0238e06de3b/django/db/migrations/state.py#L95
ProjectState.__init__()] now, then, instead of checking that `real_apps`
is a set and converting it to a set if not, it can just `assert` that it's
a set when non-`None`. (Presumably the construction of new `ProjectState`
objects is part of Django's internal API.) I had made this comment on the
PR, but it wasn't important enough to hold up the PR because another PR
was depending on it getting merged.

--
Ticket URL: <https://code.djangoproject.com/ticket/33014>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Aug 12, 2021, 1:40:44 AM8/12/21
to django-...@googlegroups.com
#33014: ProjectState.__init__() can assume its real_apps argument is a set
--------------------------------------+------------------------------------

Reporter: Chris Jerdonek | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by Carlton Gibson):

* stage: Unreviewed => Accepted


Comment:

Hey Chris.

I'm a bit ''Meh'' about the suggestion here but I'll accept so you can
make the PR for review. (If the others like it...)

> ...part of Django's internal API

I'm forever amazed what parts of Django folks out there are using. Perhaps
not this. :)

Thanks.

--
Ticket URL: <https://code.djangoproject.com/ticket/33014#comment:1>

Django

unread,
Aug 12, 2021, 2:07:53 AM8/12/21
to django-...@googlegroups.com
#33014: ProjectState.__init__() can assume its real_apps argument is a set
--------------------------------------+------------------------------------

Reporter: Chris Jerdonek | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by Chris Jerdonek):

* has_patch: 0 => 1


Comment:

Thanks for entertaining this, Carlton. On the plus side, if people //are//
using it, at least now they'll have a way of being alerted so they can
take advantage of Mariusz's improvement and avoid unneeded set
conversions...

PR: https://github.com/django/django/pull/14765

--
Ticket URL: <https://code.djangoproject.com/ticket/33014#comment:2>

Django

unread,
Aug 14, 2021, 11:26:48 AM8/14/21
to django-...@googlegroups.com
#33014: ProjectState.__init__() can assume its real_apps argument is a set
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: Chris
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Component: Migrations | Version: dev

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by David Smith):

* owner: nobody => Chris Jerdonek
* status: new => assigned


--
Ticket URL: <https://code.djangoproject.com/ticket/33014#comment:3>

Django

unread,
Aug 19, 2021, 4:08:52 AM8/19/21
to django-...@googlegroups.com
#33014: ProjectState.__init__() can assume its real_apps argument is a set
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: Chris
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):

* stage: Accepted => Ready for checkin


--
Ticket URL: <https://code.djangoproject.com/ticket/33014#comment:4>

Django

unread,
Aug 19, 2021, 6:31:01 AM8/19/21
to django-...@googlegroups.com
#33014: ProjectState.__init__() can assume its real_apps argument is a set
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: Chris
Type: | Jerdonek
Cleanup/optimization | Status: closed
Component: Migrations | Version: dev
Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):

* status: assigned => closed
* resolution: => fixed


Comment:

In [changeset:"7800596924375978df3ca995df4c3466f1cd168e" 78005969]:
{{{
#!CommitTicketReference repository=""
revision="7800596924375978df3ca995df4c3466f1cd168e"
Fixed #33014 -- Made ProjectState raise exception when real_apps argument
is not a set.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/33014#comment:5>

Reply all
Reply to author
Forward
0 new messages