Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#21338 closed Bug (needsinfo)

Can't use the same class name for proxy models

Reported by: srenskiy@… Owned by: nobody
Component: Database layer (models, ORM) Version: 1.4
Severity: Normal Keywords: model
Cc: srenskiy@… Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

If models are in separate modules but have same name get_model() always returns first occurred model.

Change History (6)

comment:1 Changed 4 years ago by srenskiy@…

Type: New featureBug

comment:2 Changed 4 years ago by Tim Graham

Has patch: set

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

comment:3 Changed 4 years ago by Shai Berger

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?

comment:4 Changed 4 years ago 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.

comment:5 Changed 4 years ago by Shai Berger

Resolution: needsinfo
Status: newclosed

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.

comment:6 Changed 4 years ago 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".

Note: See TracTickets for help on using tickets.
Back to Top