Re: [Django] #31499: Store ModeState.fields into a dict.

[Django]

#31499: Store ModeState.fields into a dict.
-+-
 Reporter:  Simon Charette   |Owner:  Simon
 Type:   |  Charette
  Cleanup/optimization   |   Status:  closed
Component:  Migrations   |  Version:  master
 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:  0|UI/UX:  0
-+-
Changes (by Mariusz Felisiak ):

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


Comment:

 In [changeset:"06889d62063f0d12aaf618101dfc1b07333117be" 06889d62]:
 {{{
 #!CommitTicketReference repository=""
 revision="06889d62063f0d12aaf618101dfc1b07333117be"
 Fixed #31499 -- Stored ModelState.fields into a dict.

 This allows the removal of its O(n) .get_field_by_name method and many
 other awkward access patterns.

 While fields were initially stored in a list to preserve the initial
 model definiton field ordering the auto-detector doesn't take field
 ordering into account and no operations exists to reorder fields of a
 model.

 This makes the preservation of the field ordering completely superflous
 because field reorganization after the creation of the model state
 wouldn't be taken into account.
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.d4f9235b1431a5d5ee95785b62b39e92%40djangoproject.com.

Re: [Django] #31499: Store ModeState.fields into a dict.

[Django]

#31499: Store ModeState.fields into a dict.
-+-
 Reporter:  Simon Charette   |Owner:  Simon
 Type:   |  Charette
  Cleanup/optimization   |   Status:  assigned
Component:  Migrations   |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by felixxm):

 * owner:  nobody => Simon Charette
 * status:  new => assigned
 * stage:  Unreviewed => Accepted


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.e0872b79feacdcd0759c4482347da1b5%40djangoproject.com.

Re: [Django] #31499: Store ModeState.fields into a dict.

[Django]

#31499: Store ModeState.fields into a dict.
-+-
 Reporter:  Simon Charette   |Owner:  nobody
 Type:   |   Status:  new
  Cleanup/optimization   |
Component:  Migrations   |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Simon Charette):

 * has_patch:  0 => 1


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.7a90046939cf7ac9b5de7afcfba01199%40djangoproject.com.

[Django] #31499: Store ModeState.fields into a dict.

[Django]

#31499: Store ModeState.fields into a dict.
+
   Reporter:  Simon Charette|  Owner:  nobody
   Type:  Cleanup/optimization  | Status:  new
  Component:  Migrations|Version:  master
   Severity:  Normal|   Keywords:
   Triage Stage:  Unreviewed|  Has patch:  0
Needs documentation:  0 |Needs tests:  0
Patch needs improvement:  0 |  Easy pickings:  0
  UI/UX:  0 |
+
 `ModeState` initially stored its `fields` into a `List[Tuple[str,
 models.Field]]` because
 
[https://github.com/django/django/blob/290eb987644c31aabcfd52d81bd5adcf7a1972aa/django/db/migrations/state.py#L368-L370
 it wanted to preserve ordering].

 However the auto-detector doesn't consider field re-ordering as a state
 change and Django doesn't support table column reordering in the first
 place. The only reason I'm aware of for keeping field ordering is to
 generate model forms out of them which is unlikely to happen during
 migrations and if it was the case the only the order in which field are
 ordered and validated would change if `Meta.fields = '__all__` is used
 [https://docs.djangoproject.com/en/3.0/topics/forms/modelforms/#selecting-
 the-fields-to-use which is discouraged].

 Given storing fields this way results in awkward and inefficient lookup by
 name for no apparent benefits and that `dict` now preserves insertion
 ordering I suggest we switch `ModelState.fields` to `Dict[str,
 models.Field]`. I suggest we do the same for `ModelState.indexes` and
 `.constraints` since they suggest from the same awkwardness which was
 likely cargo culted from `ModelState.fields` design decision.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/052.063a0371b8e741ae026debc2dac0c604%40djangoproject.com.