[Django] #21338: Can't use the same class name for proxy models
Django
----------------------------------------------+--------------------
Reporter: srenskiy@… | Owner: nobody
Type: New feature | Status: new
Component: Database layer (models, ORM) | Version: 1.4
Severity: Normal | Keywords: model
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
If models are in separate modules but have same name `get_model()` always
returns first occurred model.
--
Ticket URL: <https://code.djangoproject.com/ticket/21338>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: srenskiy@… | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.4
(models, ORM) | Resolution:
Severity: Normal | Triage Stage:
Keywords: model | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by srenskiy@…):
* needs_better_patch: => 0
* type: New feature => Bug
* needs_tests: => 0
* needs_docs: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/21338#comment:1>
Django
Reporter: srenskiy@… | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.4
(models, ORM) | Resolution:
Severity: Normal | Triage Stage:
Keywords: model | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timo):
* has_patch: 0 => 1
Comment:
This seems unlikely to deserve a backport to 1.6, but the patch may apply
cleanly to master as well:
https://github.com/django/django/pull/1817
--
Ticket URL: <https://code.djangoproject.com/ticket/21338#comment:2>
Django
Reporter: srenskiy@… | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.4
(models, ORM) | Resolution:
Severity: Normal | Triage Stage:
Keywords: model | Unreviewed
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by shai):
This ticket asks that proxy models will be able to exist in the same app,
and with the same name, as non-proxy models. This doesn't make sense to me
-- if it is valid, then why only for proxy models?
Was the intention only to allow it if the same-named model is actually the
proxied model? As far as I can see, if your app has concrete models x.A
and x.B, the patch allows you to add another model y.A that proxies x.B
(invalidating the note in the PR that for model creation, it doesn't
matter if you pick the proxy or the concrete model).
Many parts in Django assume there's only one model with a given name in an
app. Changing that is quite major, and I really don't see the use-case. It
seems much more sensible, either to give the proxy a different name, or
put it in a different app.
Can you explain the need?
--
Ticket URL: <https://code.djangoproject.com/ticket/21338#comment:3>
Django
Reporter: srenskiy@… | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.4
(models, ORM) | Resolution:
Severity: Normal | Triage Stage:
Keywords: model | Unreviewed
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by srenskiy@…):
Most likely you will never want to have same name for non-proxy models.
Also it completely will brake Django models mechanism. That's why same
name only for proxy models which are (by Liskov Principe) interchangeable
in most cases. In other cases it's up to developer decide to use same
names for proxy models or not.
By "x" in x.A you mean module or app? Concrete models will never includes
module name (only proxy): proxy_model_module = is_proxy and module or
None. But you have point, in `register_models()` there are may be cases
with lost proxy models (when concrete model overrides record of first
occurred proxy model inheriting concrete one or another). I think this is
repairable and testable. I'l try to do it later on this week.
The need is simple: python supports it why Django not? It's more practical
to have `app_store.Receipt` and `google_play.Receipt` then
`app_store.AppStoreReceipt` and `google_play.GooglePlayReceipt`.
--
Ticket URL: <https://code.djangoproject.com/ticket/21338#comment:4>
Django
Reporter: srenskiy@… | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 1.4
Severity: Normal | Triage Stage:
Keywords: model | Unreviewed
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by shai):
* status: new => closed
* resolution: => needsinfo
Comment:
By 'x' in x.A I mean a module -- if you want two models with the same name
in the same module, I think we would flat out reject it.
Models in the same app having the same name only makes partial sense if
they are in separate modules -- that is, your app `app` has some models
defined in `app.x` and some in `app.y` (or, more likely, `app.models.x`
and `app.models.y`).
What I meant was not about "lost proxy models", but simply that you claim
in the patch that, for model creation, when you pick up model `A` from app
`app`, it doesn't matter if you pick the concrete model or the proxy. Even
in the way you intended, that is not generally true (e.g. the proxy may
use more stringent validations, and cause instance creation to fail when
the parameters are valid for the concrete model). But my point was that
nothing guarantees that the proxy model is indeed a proxy for the concrete
model with the same name -- it can be a proxy for a completely unrelated
model. Think about concrete classes `tools.Tool` and `drinks.ScrewDriver`,
and proxy class `tools.ScrewDriver`.
Nothing prevents you from having `app_store.Receipt` and
`google_play.Receipt`; I just don't see why these two need to be in the
same app.
Finally, the Liskov principle is not symmetric, and definitely makes no
mention of naming your subclasses the same as superclasses.
In short -- I'm entirely unconviced. Closing as needsinfo -- feel free to
re-open if you have a more compelling argument.
--
Ticket URL: <https://code.djangoproject.com/ticket/21338#comment:5>
Django
Reporter: srenskiy@… | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 1.4
(models, ORM) | Resolution: needsinfo
Severity: Normal | Triage Stage:
Keywords: model | Unreviewed
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by srenskiy@…):
I don't want use same name for models in one class. Assume we have these
"modules" package:
{{{
models
receipts
__init__.py
google_play.py
app_store.py
__init__.py
receipt.py
}}}
My propose is in fact that both modules "google_play" and "app_store" have
models with name "Receipt" which inherit base model "Receipt" from module
"receipt".
--
Ticket URL: <https://code.djangoproject.com/ticket/21338#comment:6>