Re: models.CalculatedField feature

2017-11-16 Thread ilya . kazakevich
Thank you for your intereset this on feature.

I belive this functionality should not delegete everything to database: one 
of the cooliest Django features is that models do not require database:
they could be just storage of *business* logic (that is why we call them 
*fat*). I can even fill my models from different source and logic will 
still exist:

customer = Customer(age=variable_from_csv_file)
if customer.is_allowed_to_drink: 

But I agree that reflecting *all* database features could be cumbersome. We 
can limit usage to simple stuff like "eq", "gt(e)", and "contains" and 
literals as arguments.

I also do not think that we should ask user to use lambdas:
"CalculatedField(Q(age__gte=18), lambda c: c.age >= 18)" literally violates 
DRY (one of the key Django features after all!)
But we can give user ability to provide custom lambda (for advanced cases)

Actually, my idea about lambdas was about to make calculation "lazy"
"still_actual = Event(Q(end_of_like__lt=date.today())"

I want "today" to be calculated at the moment when method is called or SQL 
query executed. Something like
"still_actual = Event(lambda expression_api: 
expression_api.expression(end_of_like__lt=date.today())"
It looks ugly, we can create some sugar for it.

I really like how SQLAlchemy does it. 
They say "It can, in theory, work with any descriptor-based expression 
system". Can we borrow it?

Ilya.

-- 
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 https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/632c3d33-48c8-437e-877f-21eeb2121d72%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: models.CalculatedField feature

2017-11-16 Thread Tom Forbes
I think without client-side Python execution of these functions it greatly
limits the use of this feature. In the cases I've seen you have a bunch of
`qs.filter(age__gt=18)` scattered around everywhere and a separate
`can_drink` function of property. Without the `can_drink` function
returning the correct result when the model is updated it's kind of
useless, you'd still end up with people still writing their own `can_drink`
functions to do so.

If we can make simple cases work well (i.e functions that do arithmetic,
and/or, simple comparisons and on simple fields) I think it would be pretty
useful. Sure, when you get into quite complex database functions, complex
fields with differing support it can get quite hard, but for
string/integer/boolean columns with simple operators that different between
databases we support?

On Thu, Nov 16, 2017 at 12:08 PM, Sjoerd Job Postmus 
wrote:

> I disagree with not being able to calculate it locally (without the
> database): such a calculation depends on other fields. What if a dependent
> field is updated, but the object is not yet saved. What should the value be?
>
> >>> c.age, c.is_adult
> 17, False
> >>> c.age = 40
> >>> c.age, c.is_adult
> 40, False
>
> Would not make sense to me.
>
> As a first step, maybe it would make sense to make it easier to support
> custom filter attribute. Right now you have to create a custom query set
> subclass implementing .filter where you intercept the value
>
> def filter(self, *args, **kwargs):
> if 'is_adult' in kwargs:
> value = kwargs.pop('is_adult')
> if value:
> kwargs['age__gte'] = 18
> else:
> kwargs['age__lt'] = 18
> return super().filter(*args, **kwargs)
>
> And that's ignoring exclude and Q, and probably a lot of other cases.
>
> Could we make that simpler first?
>
> On 16 Nov 2017 07:43, Shai Berger  wrote:
>
> On Thursday 16 November 2017 07:21:38 Josh Smeaton wrote:
> >
> > I don't agree with reimplementing lookups/transforms/funcs to produce
> the
> > python version of a SQL concept. It's a leaky abstraction. Instead, I'd
> > prefer the model author provides their python calculation:
> >
> > is_adult = models.CalculatedField(Q(age__gte=18), lambda c: c.age
> >= 18)
>
> (name shortened to fit in a 80-char line)
>
> I disagree. The way I see it, these calculated fields should essentially
> provide a declarative way to add an annotate() that's always going to be
> there. There should be only one source of truth, and that's the database;
> parallel implementations tend to disagree on fringe cases with
> disappointing
> results (e.g. consider if age above is nullable -- the database then makes
> the
> calculated field null, the python above makes it raise an exception). Or
> consider calculated fields which are aggregates -- a Python version could
> very
> easily become painfully inefficient.
>
> >
> > There may be destructuring/serialisation issues (migrations/pickle)
> using
> > lambda, unless we can ignore these model fields during deserialisation.
> >
>
> That's another (though minor) argument against lambdas here.
>
> No, the way I see it, the API should just use expressions. Of note, I
> think it
> should also support the output_field keyword. At the hand-waving,
> brainstorming
> level, I think we might, at some point, actually try to implement this by
> creating objects in the database (views on Postgres, computed columns on
> MySql, etc) -- so that the field is really available in all contexts.
>
> My 2 cents,
>
> Shai.
>
>
> --
> 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 https://groups.google.com/group/django-developers.
> To view this discussion on the web visit https://groups.google.com/d/
> msgid/django-developers/fdf87ea3-aa91-452a-8336-
> d73dc92efe8d%40email.android.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 https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CAFNZOJMQkAbonjFi%2BSjpDzEq_azpdKXJEkM3nCG0h_an%2BE0XBQ%40mail.gmail.com.
For more options, visit 

Re: models.CalculatedField feature

2017-11-16 Thread Sjoerd Job Postmus
I disagree with not being able to calculate it locally (without the database): such a calculation depends on other fields. What if a dependent field is updated, but the object is not yet saved. What should the value be?>>> c.age, c.is_adult17, False>>> c.age = 40>>> c.age, c.is_adult40, FalseWould not make sense to me.As a first step, maybe it would make sense to make it easier to support custom filter attribute. Right now you have to create a custom query set subclass implementing .filter where you intercept the valuedef filter(self, *args, **kwargs):    if 'is_adult' in kwargs:        value = kwargs.pop('is_adult')        if value:            kwargs['age__gte'] = 18        else:            kwargs['age__lt'] = 18    return super().filter(*args, **kwargs)And that's ignoring exclude and Q, and probably a lot of other cases.Could we make that simpler first? On 16 Nov 2017 07:43, Shai Berger  wrote:On Thursday 16 November 2017 07:21:38 Josh Smeaton wrote:

> 

> I don't agree with reimplementing lookups/transforms/funcs to produce the

> python version of a SQL concept. It's a leaky abstraction. Instead, I'd

> prefer the model author provides their python calculation:

> 

> is_adult = models.CalculatedField(Q(age__gte=18), lambda c: c.age >= 18)



(name shortened to fit in a 80-char line)



I disagree. The way I see it, these calculated fields should essentially 

provide a declarative way to add an annotate() that's always going to be 

there. There should be only one source of truth, and that's the database; 

parallel implementations tend to disagree on fringe cases with disappointing 

results (e.g. consider if age above is nullable -- the database then makes the 

calculated field null, the python above makes it raise an exception). Or 

consider calculated fields which are aggregates -- a Python version could very 

easily become painfully inefficient.



> 

> There may be destructuring/serialisation issues (migrations/pickle) using

> lambda, unless we can ignore these model fields during deserialisation.

> 



That's another (though minor) argument against lambdas here.



No, the way I see it, the API should just use expressions. Of note, I think it 

should also support the output_field keyword. At the hand-waving, brainstorming 

level, I think we might, at some point, actually try to implement this by 

creating objects in the database (views on Postgres, computed columns on 

MySql, etc) -- so that the field is really available in all contexts.



My 2 cents,



	Shai.








-- 
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 https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/fdf87ea3-aa91-452a-8336-d73dc92efe8d%40email.android.com.
For more options, visit https://groups.google.com/d/optout.


Re: models.CalculatedField feature

2017-11-16 Thread Marc Tamlyn
I like the idea, and have a number of concrete use cases I would use it
for. In one case I even have a custom manager which always applies a
particular `.annotate()` already, so it can be filtered and accessed on the
model.

I am inclined to agree with Shai that the fully database version should at
least be provided. If that aggregate is expensive, it could be `.defer()`ed
(this has its own problems of course). I'm not against the idea of the user
being able to provide a python implementation, but I think the database
version should be the default as it is more likely to be completely
consistent. I definitely think we should not be trying to automagically
create python versions of database functions.

Marc

On 16 November 2017 at 06:43, Shai Berger  wrote:

> On Thursday 16 November 2017 07:21:38 Josh Smeaton wrote:
> >
> > I don't agree with reimplementing lookups/transforms/funcs to produce the
> > python version of a SQL concept. It's a leaky abstraction. Instead, I'd
> > prefer the model author provides their python calculation:
> >
> > is_adult = models.CalculatedField(Q(age__gte=18), lambda c: c.age
> >= 18)
>
> (name shortened to fit in a 80-char line)
>
> I disagree. The way I see it, these calculated fields should essentially
> provide a declarative way to add an annotate() that's always going to be
> there. There should be only one source of truth, and that's the database;
> parallel implementations tend to disagree on fringe cases with
> disappointing
> results (e.g. consider if age above is nullable -- the database then makes
> the
> calculated field null, the python above makes it raise an exception). Or
> consider calculated fields which are aggregates -- a Python version could
> very
> easily become painfully inefficient.
>
> >
> > There may be destructuring/serialisation issues (migrations/pickle) using
> > lambda, unless we can ignore these model fields during deserialisation.
> >
>
> That's another (though minor) argument against lambdas here.
>
> No, the way I see it, the API should just use expressions. Of note, I
> think it
> should also support the output_field keyword. At the hand-waving,
> brainstorming
> level, I think we might, at some point, actually try to implement this by
> creating objects in the database (views on Postgres, computed columns on
> MySql, etc) -- so that the field is really available in all contexts.
>
> My 2 cents,
>
> Shai.
>
>

-- 
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 https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CAMwjO1GTx5wPxHsxOQV8LbtDWfD144f_na%2BC57hFBXeRhtS8aw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.