Re: [Django] #23444: Legacy code unnecessarily creates ContentType object for `through` model when using it in inlines

[Django]

#23444: Legacy code unnecessarily creates ContentType object for `through` model
when using it in inlines
--+
 Reporter:  a1tus |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  contrib.admin |  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 a1tus):

 * needs_better_patch:  1 => 0


--
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 post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/063.6e5c37ce5a3f7d87c67c2b24c02a750b%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #23444: Legacy code unnecessarily creates ContentType object for `through` model when using it in inlines

[Django]

#23444: Legacy code unnecessarily creates ContentType object for `through` model
when using it in inlines
--+
 Reporter:  a1tus |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  contrib.admin |  Version:  master
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  1 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  1
Easy pickings:  0 |UI/UX:  0
--+
Changes (by berkerpeksag):

 * needs_better_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 post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/063.76895ddf0df348b30cbcfc30fb3671fe%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #23444: Legacy code unnecessarily creates ContentType object for `through` model when using it in inlines

[Django]

#23444: Legacy code unnecessarily creates ContentType object for `through` model
when using it in inlines
--+
 Reporter:  a1tus |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  contrib.admin |  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 a1tus):

 * needs_better_patch:  1 => 0


Comment:

 I've updated the patch and created new ticket about `get_for_model`:
 #23687.

--
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 post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/063.74627cad25786a168188485f1c7dbebd%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #23444: Legacy code unnecessarily creates ContentType object for `through` model when using it in inlines

[Django]

#23444: Legacy code unnecessarily creates ContentType object for `through` model
when using it in inlines
--+
 Reporter:  a1tus |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  contrib.admin |  Version:  master
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  1 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  1
Easy pickings:  0 |UI/UX:  0
--+
Changes (by timgraham):

 * needs_better_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 post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/063.9e64b9ee2a6176e1e8a0086a99cd3f95%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #23444: Legacy code unnecessarily creates ContentType object for `through` model when using it in inlines

[Django]

#23444: Legacy code unnecessarily creates ContentType object for `through` model
when using it in inlines
--+
 Reporter:  a1tus |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  contrib.admin |  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
--+

Comment (by a1tus):

 Moving it to `get_for_model` might be logical. I've checked some details
 to be sure.

 At the moment syncdb/migrate creates CT objects here:
 
https://github.com/django/django/blob/master/django%2Fcontrib%2Fcontenttypes%2Fmanagement.py

 So it just calls `app_config.get_models()` and then creates corresponding
 objects.
 Those app CT objects that were not found are suggested to be deleted.

 Moving on to `get_models.__doc__`:
 {{{
 Returns a list of all installed models.

 By default, the following models aren't included:

 - auto-created models for many-to-many relations without
   an explicit intermediate table,
 - models created to satisfy deferred attribute queries,
 - models that have been swapped out.
 }}}

 So may be some other checks in addition to "being a `through` model"
 should be added there.

 And one more thing. I doesn't make much sense but some users could use
 their `through` model in admin not only in inlines and therefore they
 might need its CT object. So that change can affect them in some way.

--
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 post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/063.e4f8e769ba5a76f3fefe0ca62d87e6ff%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #23444: Legacy code unnecessarily creates ContentType object for `through` model when using it in inlines

[Django]

#23444: Legacy code unnecessarily creates ContentType object for `through` model
when using it in inlines
--+
 Reporter:  a1tus |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  contrib.admin |  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
--+

Comment (by timgraham):

 Loic, are you suggesting that we should modify `get_for_model()` so it
 doesn't create `ContentType`s for `through` models? If so, can we create a
 separate ticket and keep this one focused on deprecating
 `original_content_type_id`?

--
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 post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/063.f1a99401c4a3b70dc6e18cf63607e12d%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #23444: Legacy code unnecessarily creates ContentType object for `through` model when using it in inlines

[Django]

#23444: Legacy code unnecessarily creates ContentType object for `through` model
when using it in inlines
--+
 Reporter:  a1tus |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  contrib.admin |  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
--+

Comment (by loic):

 IMO `InlineAdminForm` is the wrong place for a fix, can't we fix it at the
 `get_for_model()` level?

 I didn't check in detail but I think `Model._meta.auto_created` can be
 used to this effect.

--
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 post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/063.ed0970e5bc17e309ad5ceeef477c91d6%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #23444: Legacy code unnecessarily creates ContentType object for `through` model when using it in inlines

[Django]

#23444: Legacy code unnecessarily creates ContentType object for `through` model
when using it in inlines
--+
 Reporter:  a1tus |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  contrib.admin |  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 a1tus):

 * has_patch:  0 => 1


Comment:

 I've made initial pull request:
 https://github.com/django/django/pull/3201

--
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 post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/063.317ba5cf6fe0c329a4088f0b5161%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #23444: Legacy code unnecessarily creates ContentType object for `through` model when using it in inlines

[Django]

#23444: Legacy code unnecessarily creates ContentType object for `through` model
when using it in inlines
--+
 Reporter:  a1tus |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  contrib.admin |  Version:  master
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  0 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  0
Easy pickings:  0 |UI/UX:  0
--+

Comment (by timgraham):

 I don't think a method is really needed, but you could use an underscore
 method (`_original_content_type_id`) and assign it to
 `original_content_type_id` in `__init__()` if you want.


 The class to use is `RemovedInDjango20Warning`, see the
 [https://docs.djangoproject.com/en/dev/internals/contributing/writing-code
 /submitting-patches/#deprecating-a-feature Deprecating a feature] guide
 for details.

--
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 post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/063.1d49c042570e5002551f6252d5d358f5%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #23444: Legacy code unnecessarily creates ContentType object for `through` model when using it in inlines

[Django]

#23444: Legacy code unnecessarily creates ContentType object for `through` model
when using it in inlines
--+
 Reporter:  a1tus |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  contrib.admin |  Version:  master
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  0 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  0
Easy pickings:  0 |UI/UX:  0
--+

Comment (by a1tus):

 Replying to [comment:3 timgraham]:
 > Deprecation sounds good, but we'd have start the process in 1.8 and I
 don't think there's any reason to accelerate it.

 I'm ready to make PR but there 2 questions.

 === 1 ===
 Should I extract `original_content_type_id` into the separate
 `InlineAdminForm` method?
 The good part is that it will make code slightly more readable.
 But what I don't like is that we'll a bit of inconsistency: for those
 forms without `original` `original_content_type_id` property didn't even
 exist.
 On the other side we still modify class that is not very good.

 === 2 ===
 I didn't undestand about deprecation warning. Which one should I use - 1.8
 or 1.9?

--
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 post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/063.5b3580bab640b1a5f2fdd71144ee7de8%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #23444: Legacy code unnecessarily creates ContentType object for `through` model when using it in inlines

[Django]

#23444: Legacy code unnecessarily creates ContentType object for `through` model
when using it in inlines
--+
 Reporter:  a1tus |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  contrib.admin |  Version:  master
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  0 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  0
Easy pickings:  0 |UI/UX:  0
--+
Changes (by timgraham):

 * version:  1.7 => master
 * stage:  Unreviewed => Accepted


Comment:

 Deprecation sounds good, but we'd have start the process in 1.8 and I
 don't think there's any reason to accelerate it.

--
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 post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/063.08f28c96080ef6e303205c455854a02e%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #23444: Legacy code unnecessarily creates ContentType object for `through` model when using it in inlines

[Django]

#23444: Legacy code unnecessarily creates ContentType object for `through` model
when using it in inlines
-+-
 Reporter:  a1tus|Owner:  nobody
 Type:   |   Status:  new
  Cleanup/optimization   |  Version:  1.7
Component:  contrib.admin|   Resolution:
 Severity:  Normal   | Triage Stage:
 Keywords:   |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by a1tus):

 Replying to [comment:1 thedrow]:
 > What if someone relied on this attribute being set? Many applications
 theme their admin app.
 > I suggest to wrap this in a property with an expedited deprecation
 process. That is, a DeprecationWarning will be issued.

 Yes, that was on my mind too when I was submitting this issue but I've
 forgotten to mention it. Thanks!

 So now we have this code:

 {{{#!python
 if original is not None:
 # Since this module gets imported in the application's root package,
 # it cannot import models from other applications at the module level.
 from django.contrib.contenttypes.models import ContentType
 self.original_content_type_id =
 ContentType.objects.get_for_model(original).pk
 }}}

 ...and we need to keep `original_content_type_id` but prevent ContentType
 creating (and raise DeprecationWarning).
 So smth like this may work (simplified):

 {{{#!python
 if original is not None:
 from django.utils.functional import cached_property

 def original_content_type_id(self):
 import warnings
 from django.contrib.contenttypes.models import ContentType
 from django.utils.deprecation import RemovedInDjango18Warning

 warnings.warn("Use absolute_url attr instead.",
 RemovedInDjango18Warning)
 return ContentType.objects.get_for_model(original).pk

 self.__class__.original_content_type_id =
 cached_property(original_content_type_id)
 }}}

--
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 post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/063.5cb6f0f7e6366b17e611a01fc1caecaa%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #23444: Legacy code unnecessarily creates ContentType object for `through` model when using it in inlines

[Django]

#23444: Legacy code unnecessarily creates ContentType object for `through` model
when using it in inlines
-+-
 Reporter:  a1tus|Owner:  nobody
 Type:   |   Status:  new
  Cleanup/optimization   |  Version:  1.7
Component:  contrib.admin|   Resolution:
 Severity:  Normal   | Triage Stage:
 Keywords:   |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by thedrow):

 * needs_better_patch:   => 0
 * needs_tests:   => 0
 * needs_docs:   => 0


Comment:

 What if someone relied on this attribute being set? Many applications
 theme their admin app.
 I suggest to wrap this in a property with an expedited deprecation
 process. That is, a DeprecationWarning will be issued.

--
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 post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/063.bf90ffa420376f368c1de074e480e08e%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.