Hi Marten,

I can understand the motivation, and the approach you've taken makes sense.
It definitely strikes me as a much better alternative to tickets like
#24288 than a setting.

https://code.djangoproject.com/ticket/24288

The DRY issue isn't a huge problem for me - it's certainly ungainly to not
have a clean way to override an abstract field, but I'm not sure I see a
particularly Pythonic alternative.

My only real concern is the potential for accidentally overriding
something. This isn't especially likely on a field like "username" on a
User model - but a more esoteric field, like "is_staff" would be easy to
add twice, because it might not be obvious that the abstract model is
providing the field that you're overriding. Some sort of safety catch
(e.g., requiring override_abstract=True in an overriding definition) might
be a way around this.

So - I think it's worth kicking around; I'm definitely interested in
hearing if others have a similar reaction.

As far as a final patch goes, we'd definitely need some tests to validate
that the model has, in fact, been overridden. I understand this is probably
a proof-of-concept patch - I'm just flagging it in case you thought "it
passes the current test suite" would be enough.

Yours,
Russ Magee %-)


On Sat, Feb 7, 2015 at 6:48 AM, Marten Kenbeek <[email protected]>
wrote:

> Hey,
>
> While fiddling with django I noticed it would be trivial to add support
> for shadowing fields from abstract base models. As abstract fields don't
> exist in the database, you simply have to remove the check that reports
> name clashes for abstract models, and no longer override the field.
>
> This would allow you to both override fields from third-party abstract
> models (e.g. change the username length of AbstractUser), and to override
> fields of common abstract models on a per-model basis. Previously you'd
> have to copy the original abstract model just to change a single field, or
> alter the field after the model definition. The first approach violates the
> DRY principle, the second approach can introduce subtle bugs if the field
> is somehow used before you changed it (rare, but it can happen).
>
> This patch
> <https://github.com/knbk/django/commit/e8347457bccaeab11f9624b01c57824f4511da6c>
>  adds
> the feature. It seems to work correctly when using the models and
> creating/applying migrations, and passes the complete test suite.
>
> What do you guys think about this feature?
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to [email protected].
> To post to this group, send email to [email protected].
> Visit this group at http://groups.google.com/group/django-developers.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/f846521f-8a16-41c7-9998-2bea7c93b3dd%40googlegroups.com
> <https://groups.google.com/d/msgid/django-developers/f846521f-8a16-41c7-9998-2bea7c93b3dd%40googlegroups.com?utm_medium=email&utm_source=footer>
> .
> For more options, visit https://groups.google.com/d/optout.
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CAJxq849Jt4t2binnyt5o4BdqSr_Lg_EBEzw4s8h9_H%2BmyvuPMQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to