Becareful with Syntax description

[moumoutte]

I've passed two day , trying to debug a bug which doesn't not exists because I was confused on the description syntax. I would like to make a update on the syntaxe describtion, motivated by mind health and clearest code.

For the moment, when we want to add a counter with filtering the syntax is like that:

'agg_type': A list of dictionary with single value inside

'count': [
    {'public_leaves': [{'field': 'private', 'value': True}]},
    {'private_leaves': #blabla }
]

This syntax can be simplify by moving the list and dictionary with unique value inside by a single dictionary with multiple value

'count': {
   'public_leaves': [{'field': 'private', 'value': True}],
   'private_leaves': #blabla 
}

Less inference is better for our mind and our code. This piece of code can be more elegant than accessing the first element into a list, just to have the name of the counter:

https://github.com/novafloss/django-aggtrigg/blob/master/django_aggtrigg/util.py#L229
https://github.com/novafloss/django-aggtrigg/blob/master/django_aggtrigg/util.py#L231

Here I'm just talking about item in agg_type description. But, It can be extended to the description of agg_type itself.

[rodo]

It less explicit for me, we do not need a dict when a list is enough, but iif you and other contributor think is more explicit with a dict I'm ok

[moumoutte]

we do not need a dict when a list is enough

I'm really surprise by your answer, because, we need a list AND as many dict as counter defined.... In my proposition, we need only one dict.

[brunobord]

I'm sorry but I agree with @moumoutte. A list with a "one-key dictionary" looks weird to me.
iterating on this structure needs two operations, along with key-mangling.

trigger_list = [
    {'foo': [{'field': 'private', 'value': True}]},
    {'bar': [{"field": "private": "value": False}]},
]
for item_dict in trigger_list:
    key = item_dict.keys()[0]  # we'll always need the first, and never any other...
    trigger_definitions = item_dict[key]
    # Do stuff

What @moumoutte is suggesting is way more direct

trigger_dict = {
    'foo': [{'field': 'private', 'value': True}],
    'bar': [{"field": "private": "value": False}],
]
for key, trigger_definitions in trigger_dict.items():
    # Do stuff

the question is: will you need "foo" to appear more than once?

[brunobord]

there are alternate syntaxes, that are still pythonic (just thought about them):

trigger_list = [
    ('foo', [{'field': 'private', 'value': True}]),
    ('bar': [{"field": "private": "value": False}]),
]
for key, definition in trigger_list:
   # to stuff

or we could use namedtuples, for more semantics: https://docs.python.org/2/library/collections.html#collections.namedtuple

[rodo]

@brunobord please do not be sorry to take time to help :)
If you ( @moumoutte and @brunobord ) are agree and think this is better I'll be ok to merge a functionnal and travis validated PR.

[moumoutte]

I would like to make an update on the syntax description combined with this issue : #51 to allow OR in filtering.

I have definitely no doubt, let the user writes his own query set is the best way. So, we can accept a callback as value on the dict description.

    'count': {
        'foo': lambda base_qs: base_qs.filter(Q(toto='tuto') | Q(toto='tutu')).exclude(pk=12).distinct()
        'bar': lambda base_qs: base_qs.filter(private=True)
}

By this way, the user can expresses his queryset as any django queryset, there is no limit about using queryset (except values() may be..) and available mehod on it.

[rodo]

Ok if you want to update the description even if I don't see why you need so complex query to maintain counters. Keep in mind a trigger may stay as little as possible, if you introduce too complex query in the function call by tje trigger you'll loose all performance benefits. If you need to do a complexe query to know a model's state it's may be that your database schemas is not well designed and django-aggtrig is not the solution, redesign your db is the one.