The general reaction seems to be a yes if we can work out the kinks, so I 
went ahead and created a ticket: https://code.djangoproject.com/ticket/24305

@Aron That's a good question. One option would be to lock certain fields, 
so that they can't be changed if they are an integral part of the model. 
That would be a simple solution, but that won't help for existing code that 
doesn't lock the fields. It won't break existing code, but it won't protect 
for errors either. The opt-in version (i.e. an 'unlock' attribute) would 
lock many fields which would otherwise be completely safe to overwrite. 

Another option would be more elaborate "requirements" for a manager or some 
methods, i.e. allow the manager to specify the necessary class of a certain 
field or a minimum length. If the modeldoesn't meet the requirements, the 
manager or some of the methods will not be inherited. While it allows for 
more control, this option would greatly increase the complexity of the 
patch and requires more from the developers of custom managers. It can also 
cause issues when the requirements aren't up-to-date with the manager's 
methods. 

We could also say that it is the users responsibility and don't provide 
special protection, in line with the fields themselves, but I guess that 
this would generally be more problematic for custom managers. It can also 
cause silent bugs when the manager's methods don't work as intended but 
won't raise an exception either, which is not a good idea imho. 

I think the locking approach would be the easiest and most pragmatic 
method. I think it's still - in part - the users responsibility to confirm 
that a field can be overridden. The Django documentation could, where 
applicable, document the requirements on fields that can be overridden, 
i.e. that an AbstractUser's username must be a CharField (which isn't 
necessarily true, just an example). 

@Loïc The bases are indeed traversed in the reversed order. 

Op zondag 8 februari 2015 08:19:57 UTC+1 schreef Loïc Bistuer:
>
> That's what we've done in Django 1.7 for Form/ModelForm (#19617, #8620), 
> and I completely agree that it should be possible to shadow fields from 
> abstract models. The docs even suggest that this may be possible in the 
> future; quoting the relevant section: "this [shadowing] is not permitted 
> for attributes that are Field instances (at least, not at the moment)." 
>
> For consistency with forms - and because we've put a great deal of 
> thinking into it - the implementation should be: you can shadow a field 
> with another field, or you can remove a field using None as a sentinel 
> value (see #22510 for more details). 
>
> I believe we have a safety net in the form of migrations, if you 
> accidentally shadow a field, migrations will pick it up, so it's unlikely 
> to go unnoticed. 
>
> I'd be quite happy to shepherd this patch. 
>
> Unit tests should cover those scenarios: 
>
> - An abstract model redefines or removes some fields from a parent 
> abstract model. 
> - A concrete model redefines or removes some fields from a parent abstract 
> model. 
> - Ensure that the standard MRO resolution applies when you do 
> Concrete(AbstractA, AbstractB) with AbstractA redefining a field from 
> AbstractB. 
>
> The last case may be tricky because if I remember well ModelBase iterates 
> through the MRO in the wrong direction (i.e. bases should be iterated over 
> in reverse). 
>
> We also need some tests to show how migrations behave. 
>
> -- 
> Loïc 
>
> > On Feb 7, 2015, at 09:33, Marten Kenbeek <marte...@gmail.com 
> <javascript:>> wrote: 
> > 
> > Hi Russ, 
> > 
> > I can see your point on accidentally overriding fields, though I'm not 
> sure I agree. In any other case, such as normal attributes and methods, 
> there is no safety net for overriding attributes either. Any model that 
> would be affected by this change would also raise an error on import 
> without the patch, so existing functional code won't be affected. On the 
> other hand, a new parameter for the field wouldn't be that much of a hassle 
> to implement or work with. I'd be interested to hear what others think 
> about this. 
> > 
> > There are more than a few questions on stack overflow that expect this 
> behaviour, even if the docs specifically mention that it won't work. If 
> users intuitively try to override fields in this manner, I think it would 
> be reasonable to allow this without any explicit arguments. 
> > 
> > We can always restrict what you can override, at least as the default 
> behaviour, e.g. so that you can only use subclasses of the original field. 
> That would make code that depends on the abstract field less prone to bugs, 
> though subtle bugs can still happen if you e.g. override the max length of 
> a field. 
> > 
> > This was indeed just a proof-of-concept. That remark was meant more like 
> "it doesn't appear to break everything". 
> > 
> > Marten 
> > 
> > Op vrijdag 6 februari 2015 23:48:55 UTC+1 schreef Marten Kenbeek: 
> > 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 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 django-develop...@googlegroups.com <javascript:>. 
> > To post to this group, send email to django-d...@googlegroups.com 
> <javascript:>. 
> > 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/072149e1-c794-446d-957b-f5fc5df87096%40googlegroups.com.
>  
>
> > 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 django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
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/1bade953-e35c-4b39-b34a-08c6d1b8be7a%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to