Ticket #16311 review please ? (admin.filter.RelatedFieldListFilter extension)

[Stan]

Hi devs,

Could some take a look at this cool pull 
requestsplease ? I know the 5 for 1 
rule but quite frankly I can't find something 
to work on (so if you want to put me on something in particular, that's 
fine :)

TL;DR :

In your *space_program* app model you have:

- a *Country* table and ~200 entries;
- an *Astronaut* table with a FK to the Country ;

In your admin, you want to be able to filter the astronauts by country but 
you don't want the whole countries to be listed since only ~10 are involved 
in the relation.
The PR add a hook in *admin.filter.RelatedFieldListFilter* and provide 
*RelatedOnlyFieldListFilter* for that purpose.


Thanks !

Stanislas.


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

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" 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/55f375e7-9edd-4a6d-ba7d-ab4b546a444c%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Why not Single Table Inheritance?

[Anssi Kääriäinen]

I think it is time to add a new model classmethod from_db() to Django.

The idea is to allow customization of object initialization when loading 
from database. Instead of calling directly model.__init__ from the queryset 
iterators, Django calls model_cls.from_db(). The default implementation 
calls just model.__init__, but by overriding from_db() it will be possible 
to do interesting things. For example:
  1) It allows for faster loading of models. See 
https://code.djangoproject.com/ticket/19501 for some benchmarks.
  2) Possibility to return polymorphic classes from querysets. For example 
for STI:
def from_db(cls, using, fields, values):
# Assume database has type column, and the class contains a 
_type_map pointing to the wanted class for each different type.
data = dict(zip(fields, values))
model_cls = cls._type_map[data['type'])
new = model_cls(**data)
new._state = ModelState(using, adding=False)
return new

  3) Allow differentiating database loading from initialization in user 
code. For example (pseudo-codeish) automatic update_fields on save():

class AutoTracking(models.Model):
fields...

@classmethod
def from_db(cls, using, fields, values):
new = super().from_db(using, fields, values)
# This step is surprisingly hard to do correctly at the moment!
# Can't use overridden __init__ or signals as they don't know 
if the model is loaded from the
# database or not.
new._old_data = dict(zip(fields, values))
return new

def save(...):
if update_fields is None:
update_fields = set()
for attr_name, v in self._old_data.items():
if getattr(self, attr_name) != v:
update_fields.add(attr_name)
super().save(...)


So, there are several advantages to adding from_db(). The only problem I 
can see with this approach is that the default model initialization code 
path will be around 5%-10% slower due to the extra from_db() call for each 
row. To me that isn't big enough slowdown to worry about. In addition, as 
shown in #19501, usage of from_db() allows for significantly faster model 
loading for special cases.

The patches in #19501 are IMO too complex. The patches try to automatically 
detect when we can skip calling model.__init__. Instead we should just add 
the from_db() hook without any fast-path automation.

Any thoughts on this idea?

 - Anssi

On Friday, May 16, 2014 5:06:18 PM UTC+3, Carl Meyer wrote:
>
> On 05/16/2014 04:46 AM, Shai Berger wrote: 
> > On Monday 12 May 2014 12:27:01 Thomas Güttler wrote: 
> >> Single Table Inheritance is used by ruby-on-rails and SQLAlchemy. 
> >> 
> >> Are there reasons why it is used in django? 
> >> 
> > 
> > Essentially, STI is a form of database denormalization. I think Django 
> should 
> > not encourage this. 
>
> I agree. 
>
> >> I would love to see a polymorphic inheritance solution in django. 
> >> 
> > 
> > Just to spell things out: You want to have models A, B(A), C(A) and 
> D(B), so 
> > that list(A.objects.all()) returns a list of objects of different types. 
> > 
> > This sounds like a good idea in general, but there are devils in the 
> > implementation details. In particular, I'd like to separate this issue 
> from 
> > the issue of STI -- polymorphism is an issue of processing the retrieved 
> > records, and need not be tightly coupled to the database layout. The 
> > polymorphism solution should work whether the records are fetched by the 
> > equivalent of A.objects.all() or 
> > A.objects.select_related(child_classes).all(). 
> > 
> > I think you can sort-of achieve STI by doing it the other way around: 
> Define 
> > all your hierarchy as abstract models, with one concrete model 
> inheriting all 
> > of them (I suspect any STI implementation in Django would have to do 
> something 
> > very similar "behind the scenes"). Pros and cons (as well as testing if 
> this 
> > actually works) are left as an exercise to the reader. 
> > 
> >> I know that there are third party apps which provide this, but 
> >> something like this should be in the core. 
> >> 
> > 
> > If you ignore STI, I think it is quite straightforward to solve this 
> with a 
> > parent model class which adds a type field, and manager methods to add 
> the 
> > select_related calls and "interpret" the type field properly; so I don't 
> see an 
> > immediate need for inclusion in core. 
>
> You don't even need the "type" field, you can just select_related all 
> the subclasses and then test when iterating over the queryset which one 
> exists for each record. This is what InheritanceManager in 
> django-model-utils does. 
>
> I don't see a need to have this in core either. It seems to me almost a 
> perfect example of the occasionally-useful-but-not-essential 
> functionality that is well served by third-party packages. (IMO con

Re: Why not Single Table Inheritance?

[Shai Berger]

On Thursday 22 May 2014 11:05:24 Anssi Kääriäinen wrote:
> I think it is time to add a new model classmethod from_db() to Django.
> 
> The idea is to allow customization of object initialization when loading
> from database. Instead of calling directly model.__init__ from the queryset
> iterators, Django calls model_cls.from_db(). The default implementation
> calls just model.__init__, but by overriding from_db() it will be possible
> to do interesting things. 

[...]

> 
> Any thoughts on this idea?
> 

Instinctively -- isn't it possible to achieve the same things today by 
overriding __new__ ?

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" 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/201405221113.53601.shai%40platonix.com.
For more options, visit https://groups.google.com/d/optout.

Re: Why not Single Table Inheritance?

[Anssi Kääriäinen]

On 05/22/2014 11:13 AM, Shai Berger wrote:

Any thoughts on this idea?


Instinctively -- isn't it possible to achieve the same things today by
overriding __new__ ?
My understanding is that achieving all the same things isn't possible. 
The problem is that inside __new__ it is impossible to know if the call 
to __new__ was made from database loading or from user code. It also 
seems that it is impossible to alter the args and kwargs passed to 
__init__(). In addition if one wants for some reason (speed, or not 
invoking __setattr__) to assign values directly to the __dict__ of the 
new class, then __new__() doesn't seem to offer any way to do that.


It is true that STI is likely possible with usage of __new__() as long 
as you don't want to change the arguments to the __init__ call of the 
created object.


As a side note I think direct assignment to __dict__ on model loading 
would be a better design than the current __init__ call. For example 
Django needs to do some pretty crazy stuff  in __init__() to support 
deferred field loading. With direct __dict__ assignment deferred model 
creation is trivial. Also, loading from the database is a form of 
deserialization, and when deserializing you want to load the model as it 
were saved. The way to do this is to avoid __init__, __setattr__ and 
descriptor __set__ calls. To avoid those the values should be assigned 
directly to the __dict__ of the object. This is also used by Python's 
deserialization. Of course, thinking about this is mostly academic. 
Changing the way model loading from database is done has severe 
backwards compatibility issues. Even django-core relies on descriptor 
calls in some case. As an example to_python() method of custom fields is 
called through a descriptor.


 - Anssi

--
You received this message because you are subscribed to the Google Groups "Django 
developers" 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/537DBD38.4080604%40thl.fi.
For more options, visit https://groups.google.com/d/optout.

Re: Schema tests and .extra queryset method

[Maximiliano Robaina]

El miércoles, 21 de mayo de 2014 21:06:22 UTC-3, Russell Keith-Magee 
escribió:
>
> Hi Maximiliano,
>
> Thanks for letting us know. If you find a problem like this, the best way 
> to report it is to open a ticket in Trac. That way we have a permanent 
> record of the existence of the issue, and we can mark the issue as resolved 
> when a fix is applied to the source tree. It also allows us to flag and 
> triage specific issues so that we know they get addressed before a release.
>
> Yours,
> Russ Magee %-)
>

Done! 
https://code.djangoproject.com/ticket/22683#ticket



 

>
>
> On Thu, May 22, 2014 at 6:38 AM, Maximiliano Robaina 
> 
> > wrote:
>
>> Hi,
>>
>> Running schema tests I caught an issue, more precisely, 
>> in test_add_field_default_transform.
>> At the end, this test method is doing:
>>
>> self.assertEqual(Author.objects.extra(where=["thing = 1"]).count(), 2)
>>
>> The problem here is what in this where clause, the "thing" field must be 
>> quoted.
>>
>> In firebird :
>>
>> SELECT * FROM AUTHOR WHERE thing = 1   <-- Here thing (in lowercase) 
>> and THING (in uppercase) are equivalent, are the same object
>>
>> is different of:
>>
>> SELECT * FROM AUTHOR WHERE "thing" = 1   <--  field is quoted
>>
>> For a more generic test I think we need to avoid use .extra method or 
>> another raw sql statement.
>>
>>
>>
>>
>>
>>  -- 
>> You received this message because you are subscribed to the Google Groups 
>> "Django developers" group.
>> To unsubscribe from this group and stop receiving emails from it, send an 
>> email to django-develop...@googlegroups.com .
>> To post to this group, send email to 
>> django-d...@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/1d03c3b7-8f8e-4794-97bc-73cfcb699d07%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" 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/fa541d6a-56c7-4d52-816e-40c84b388e6b%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Schema tests and .extra queryset method

[Maximiliano Robaina]

El jueves, 22 de mayo de 2014 03:24:12 UTC-3, Shai Berger escribió:
>
> Hi Maximiliano, 
>
> The issue of case in quoted and un-quoted names in SQL is indeed a sad 
> mess, 
> bad decisions made in the 1960s that we still need to live with today. 
>
> On Wednesday 21 May 2014 15:38:57 Maximiliano Robaina wrote: 
> > 
> > self.assertEqual(Author.objects.extra(where=["thing = 1"]).count(), 2) 
> > 
> > The problem here is what in this where clause, the "thing" field must be 
> > quoted. 
> > 
> > In firebird : 
> > 
> > SELECT * FROM AUTHOR WHERE thing = 1   <-- Here thing (in lowercase) 
> > and THING (in uppercase) are equivalent, are the same object 
> > 
> > is different of: 
> > 
> > SELECT * FROM AUTHOR WHERE "thing" = 1   <--  field is quoted 
> > 
>
> Quoting it uncoditionally would break the test on Oracle (which, by 
> default, 
> turns all the names to uppercase). There is a feature-flag for it... 
>

How this feature flag should works?
I see that Oracle backend is always making an upper in 
DatabaseOperations.quote_name, therefore, how does Oracle pass test like 
[1] where it is using field name in lowecase [2] ?

[1] https://github.com/django/django/blob/1.7b4/tests/schema/tests.py#L152
[2] https://github.com/django/django/blob/1.7b4/tests/schema/tests.py#L172


 

>
> > For a more generic test I think we need to avoid use .extra method or 
> > another raw sql statement. 
>
> ...but I agree -- an "extra" should not be needed at all in this specific 
> test. 
> The Author model has the added column as a field. 
>
> Shai. 
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" 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/adadfd6d-143c-42dc-85e3-af38fcbd21b3%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.