Re: is_valid as property

[Aymeric Augustin]

Hello Sven,

> On 12 Oct 2016, at 12:21, Sven R. Kunze  wrote:
> 
> Am Dienstag, 11. Oktober 2016 17:51:20 UTC+2 schrieb Aymeric Augustin:
> 
> The API for displaying errors is form.errors.
> 
> If the documentation implies otherwise, please accept my apologies!
> 
> No problem. I now did read the documentation 
> (https://docs.djangoproject.com/es/1.10/ref/forms/api/) to be 100% sure.
> 
> It says: "The primary task of a Form object is to validate data. With a bound 
> Form instance, call the is_valid() method to run validation and return a 
> boolean designating whether the data was valid:" and furthermore "You can 
> access errors without having to call is_valid() first. The form’s data will 
> be validated the first time either you call is_valid() or access errors."
> 
> So, the docs are actually pretty clear about what will happen (how to code 
> works), but not so much about the intended usage.

The intended usage is shown here: 
https://docs.djangoproject.com/en/1.10/topics/forms/#the-view. The document 
goes on to describe various ways to render errors in template.

Sorry, I overreacted because I failed to imagine other use cases. I withdraw my 
objections to using is_valid() in templates. While it’s (likely) not intended, 
it isn’t unrealistic either.

I’m still -0 on changing is_valid to be a property because I still think it’s 
less appropriate (validation can be expensive) and the benefits are more 
limited than they were for is_authenticated / is_anonymous.

-- 
Aymeric.

-- 
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/C04506DD-2176-4C29-A90A-206A644216BE%40polytechnique.org.
For more options, visit https://groups.google.com/d/optout.

Re: is_valid as property

[Sven R. Kunze]

I agree with Tim. I don't think it's overly necessary.

FWIW, form.validate actually is form.full_clean.

Am Dienstag, 11. Oktober 2016 19:04:51 UTC+2 schrieb Tim Graham:
>
> I don't think requiring every Django project to rewrite their views from:
>
> if form.is_valid():
>
> to 
>
> form.validate()
> if form.is_valid:
>
> is practical.
>
> About changing form.errors not to validate the form, I'm not sure there's 
> much benefit there either. When writing tests, I often write: 
> self.assertEqual(form.errors, {...}) without calling is_valid().
>
> On Tuesday, October 11, 2016 at 11:58:50 AM UTC-4, ludovic coues wrote:
>>
>> If we choose to move the call to full_clean from errors to is_valid 
>> with a deprecation period, maybe we could rename is_valid to validate. 
>> is_valid could be kept  as a property, raising error if called before 
>> validate like error would be after the change. 
>>
>> 2016-10-11 17:50 GMT+02:00 Aymeric Augustin 
>> : 
>> > On 11 Oct 2016, at 14:52, Sven R. Kunze  wrote: 
>> > 
>> > I did never say "let's do business logic in templates". AFAIK, 
>> displaying 
>> > errors (or preparing for it HTML-structure-wise) requires knowing 
>> whether a 
>> > form is invalid or not. 
>> > 
>> > 
>> > The API for displaying errors is form.errors. 
>> > 
>> > If the documentation implies otherwise, please accept my apologies! 
>> > 
>> > -- 
>> > Aymeric. 
>> > 
>> > -- 
>> > 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. 
>> > To post to this group, send email to django-d...@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/810B668F-D4D3-4C39-90CF-7357DF029921%40polytechnique.org.
>>  
>>
>> > 
>> > For more options, visit https://groups.google.com/d/optout. 
>>
>>
>>
>> -- 
>>
>> Cordialement, Coues Ludovic 
>> +336 148 743 42 
>>
>

-- 
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/231cc4f9-8dd9-4c1e-bf40-0786c2d26cb6%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: is_valid as property

[Sven R. Kunze]

Am Dienstag, 11. Oktober 2016 17:51:20 UTC+2 schrieb Aymeric Augustin:
>
> On 11 Oct 2016, at 14:52, Sven R. Kunze  
> wrote:
>
> I did never say "let's do business logic in templates". AFAIK, 
> *displaying* errors (or preparing for it HTML-structure-wise) requires 
> *knowing* whether a form is invalid or not.
>
>
> The API for displaying errors is form.errors.
>
> If the documentation implies otherwise, please accept my apologies!
>

No problem. I now did read the documentation 
(https://docs.djangoproject.com/es/1.10/ref/forms/api/) to be 100% sure.

It says: "The primary task of a Form object is to validate data. With a 
bound Form instance, call the is_valid() method to run validation and 
return a boolean designating whether the data was valid:" and furthermore 
"You can access errors without having to call is_valid() first. The form’s 
data will be validated the first time either you call is_valid() or access 
errors."

So, the docs are actually pretty clear about what will happen (how to code 
works), but not so much about the intended usage.

I for one, however, think that this is also not necessary as the names of 
those properties are pretty clear in their meaning.

-- 
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/3e6fe46a-2eaf-43a4-afe5-61b58c6b5258%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: is_valid as property

[Tim Graham]

I don't think requiring every Django project to rewrite their views from:

if form.is_valid():

to 

form.validate()
if form.is_valid:

is practical.

About changing form.errors not to validate the form, I'm not sure there's 
much benefit there either. When writing tests, I often write: 
self.assertEqual(form.errors, {...}) without calling is_valid().

On Tuesday, October 11, 2016 at 11:58:50 AM UTC-4, ludovic coues wrote:
>
> If we choose to move the call to full_clean from errors to is_valid 
> with a deprecation period, maybe we could rename is_valid to validate. 
> is_valid could be kept  as a property, raising error if called before 
> validate like error would be after the change. 
>
> 2016-10-11 17:50 GMT+02:00 Aymeric Augustin 
> : 
> > On 11 Oct 2016, at 14:52, Sven R. Kunze  
> wrote: 
> > 
> > I did never say "let's do business logic in templates". AFAIK, 
> displaying 
> > errors (or preparing for it HTML-structure-wise) requires knowing 
> whether a 
> > form is invalid or not. 
> > 
> > 
> > The API for displaying errors is form.errors. 
> > 
> > If the documentation implies otherwise, please accept my apologies! 
> > 
> > -- 
> > Aymeric. 
> > 
> > -- 
> > 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 . 
> > To post to this group, send email to django-d...@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/810B668F-D4D3-4C39-90CF-7357DF029921%40polytechnique.org.
>  
>
> > 
> > For more options, visit https://groups.google.com/d/optout. 
>
>
>
> -- 
>
> Cordialement, Coues Ludovic 
> +336 148 743 42 
>

-- 
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/e9660388-1ea1-4c6a-9de6-12ef392eab91%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: is_valid as property

[ludovic coues]

If we choose to move the call to full_clean from errors to is_valid
with a deprecation period, maybe we could rename is_valid to validate.
is_valid could be kept  as a property, raising error if called before
validate like error would be after the change.

2016-10-11 17:50 GMT+02:00 Aymeric Augustin
:
> On 11 Oct 2016, at 14:52, Sven R. Kunze  wrote:
>
> I did never say "let's do business logic in templates". AFAIK, displaying
> errors (or preparing for it HTML-structure-wise) requires knowing whether a
> form is invalid or not.
>
>
> The API for displaying errors is form.errors.
>
> If the documentation implies otherwise, please accept my apologies!
>
> --
> Aymeric.
>
> --
> 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/810B668F-D4D3-4C39-90CF-7357DF029921%40polytechnique.org.
>
> For more options, visit https://groups.google.com/d/optout.



-- 

Cordialement, Coues Ludovic
+336 148 743 42

-- 
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/CAEuG%2BTbhSrvLdbBmBPMoXa6NZ1dT4bs70QEg8VHFR%3DT6rVb5Dw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Re: is_valid as property

[Aymeric Augustin]

> On 11 Oct 2016, at 14:52, Sven R. Kunze  wrote:
> 
> I did never say "let's do business logic in templates". AFAIK, displaying 
> errors (or preparing for it HTML-structure-wise) requires knowing whether a 
> form is invalid or not.

The API for displaying errors is form.errors.

If the documentation implies otherwise, please accept my apologies!

-- 
Aymeric.

-- 
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/810B668F-D4D3-4C39-90CF-7357DF029921%40polytechnique.org.
For more options, visit https://groups.google.com/d/optout.

Re: is_valid as property

[Michal Petrucha]

On Tue, Oct 11, 2016 at 04:28:40PM +0200, ludovic coues wrote:
> First time you access form.errors, it will call form.full_clean.
> 
> But after looking at the code and not simply reading the doc, I agree
> that is_valid should be a property. After the first call to is_valid,
> the value won't change, even if a field is set to a non-valid value.

Personally, I'd argue that the opposite should be true; is_valid
should remain a method, and instead, we should change errors to no
longer trigger full validation on first access. One possibility which
would make sense to me would be that form.is_valid() could be the only
supported way to trigger full validation, and form.errors could raise
an error if it's accessed before calling is_valid first. The reasons I
see for this change have been articulated pretty clearly by several
people in this thread already...

Of course, this would have to go through a full deprecation period,
since the current behavior of form.errors is documented [0]...

Cheers,

Michal


[0]: 
https://docs.djangoproject.com/en/1.10/ref/forms/api/#django.forms.Form.errors

-- 
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/20161011143736.GA12099%40konk.org.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: Digital signature

Re: is_valid as property

[ludovic coues]

First time you access form.errors, it will call form.full_clean.

But after looking at the code and not simply reading the doc, I agree
that is_valid should be a property. After the first call to is_valid,
the value won't change, even if a field is set to a non-valid value.

2016-10-11 15:49 GMT+02:00 Sven R. Kunze :
> Am Dienstag, 11. Oktober 2016 15:03:21 UTC+2 schrieb ludovic coues:
>>
>> If I remember correctly, form.is_valid do the actual form validation
>> and return false in case of error.
>
>
> I don't know what counts as "actual form validation" but form.is_valid is
> "just" a wrapper on form.errors which is a property.
>
> cf.
> https://github.com/django/django/blob/3c447b108ac70757001171f7a4791f493880bf5b/django/forms/forms.py#L164
>
> --
> 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/bc3d0c3a-024a-4149-afde-6f34c99f53d2%40googlegroups.com.
>
> For more options, visit https://groups.google.com/d/optout.



-- 

Cordialement, Coues Ludovic
+336 148 743 42

-- 
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/CAEuG%2BTY-SjFvX8TgT%3Dd57fX0Z6P_bkp3nfLTUmRWKiC-US-qrQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Re: is_valid as property

[Sven R. Kunze]

Am Dienstag, 11. Oktober 2016 15:03:21 UTC+2 schrieb ludovic coues:
>
> If I remember correctly, form.is_valid do the actual form validation 
> and return false in case of error. 
>

I don't know what counts as "actual form validation" but form.is_valid is 
"just" a wrapper on form.errors which is a *property*. 

cf. 
https://github.com/django/django/blob/3c447b108ac70757001171f7a4791f493880bf5b/django/forms/forms.py#L164

-- 
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/bc3d0c3a-024a-4149-afde-6f34c99f53d2%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: is_valid as property

[ludovic coues]

If I remember correctly, form.is_valid do the actual form validation
and return false in case of error.

In your template, you can use {% if form.errors %} to check if the
form has any error. It's a dict with field as key and list of errors
as value.

2016-10-11 14:52 GMT+02:00 Sven R. Kunze :
> Am Dienstag, 11. Oktober 2016 14:16:50 UTC+2 schrieb Aymeric Augustin:
>>
>> Hello Sven,
>>
>> On 11 Oct 2016, at 14:07, Sven R. Kunze  wrote:
>>
>> I took a sample of 4 of my colleagues and asked them what the problem with
>> expressions like {% if form.is_valid %} is. They have no idea. They said "it
>> might not make sense in some cases but when it makes sense, it doesn't look
>> very terrible me”.
>>
>>
>> Can you show a non-contrived example, predating this discussion, of a
>> situation where this pattern make sense?
>
>
>
> For displaying some error indicators, changing backgrounds to signal colors
> (like red), etc. I don't know what counts as a non-contrived example to you.
>
>
>>
>>
>> So, what's the real problem here?
>>
>>
>> The real problem is that templates aren’t the correct place to implement
>> business logic such as form validation.
>
>
>
> I did never say "let's do business logic in templates". AFAIK, displaying
> errors (or preparing for it HTML-structure-wise) requires knowing whether a
> form is invalid or not.
>
> When there's some logic to do, that's fine. But when some developer requires
> a simple boolean flag to know whether a form is valid or not, why shouldn't
> he use form.is_valid? That looks to me like making things more complicated
> than necessary.
>
>
> Also, think about GET forms + AJAX which do not POST after redirect at all
> (e.g. search form + result table in separate AJAX containers). There you
> will re-use the same template for success and failure.
>
> --
> 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/4ffb3d8c-43fa-4bc1-9762-eae79334af28%40googlegroups.com.
>
> For more options, visit https://groups.google.com/d/optout.



-- 

Cordialement, Coues Ludovic
+336 148 743 42

-- 
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/CAEuG%2BTZ7aFginSqg0aOrNTTH6CbneO3AXt6QSUvwafCfx01%2B9w%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Re: is_valid as property

[Sven R. Kunze]

Am Dienstag, 11. Oktober 2016 14:16:50 UTC+2 schrieb Aymeric Augustin:
>
> Hello Sven,
>
> On 11 Oct 2016, at 14:07, Sven R. Kunze  
> wrote:
>
> I took a sample of 4 of my colleagues and asked them what the problem with 
> expressions like {% if form.is_valid %} is. They have no idea. They said 
> "it might not make sense in some cases but when it makes sense, it doesn't 
> look very terrible me”.
>
>
> Can you show a non-contrived example, predating this discussion, of a 
> situation where this pattern make sense?
>


For displaying some error indicators, changing backgrounds to signal colors 
(like red), etc. I don't know what counts as a non-contrived example to you.

 

>
> So, what's the real problem here?
>
>
> The real problem is that templates aren’t the correct place to implement 
> business logic such as form validation.
>


I did never say "let's do business logic in templates". AFAIK, *displaying* 
errors (or preparing for it HTML-structure-wise) requires *knowing* whether 
a form is invalid or not.

When there's some logic to do, that's fine. But when some developer 
requires a simple boolean flag to know whether a form is valid or not, why 
shouldn't he use form.is_valid? That looks to me like making things more 
complicated than necessary.


Also, think about GET forms + AJAX which do not POST after redirect at all 
(e.g. search form + result table in separate AJAX containers). There you 
will re-use the same template for success and failure.

-- 
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/4ffb3d8c-43fa-4bc1-9762-eae79334af28%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: is_valid as property

[Sven R. Kunze]

Am Dienstag, 11. Oktober 2016 14:11:28 UTC+2 schrieb Sven R. Kunze:
>
> Am Donnerstag, 29. September 2016 21:25:44 UTC+2 schrieb Sjoerd Job 
> Postmus:
>>
>> Thinking of an alternative route:
>>
>> It seems to me that the default `method.__bool__` is undesirable in 
>> Jinja2 templates. I do not know Jinja2 well enough, but maybe they could 
>> benefit from a patch where `if`-statements give a warning/error when the 
>> expression is a callable (with the default `FunctionType.__bool__`?
>>
>> This would solve the issue not just for the methods you mention, but more 
>> in general.
>>
>> [Or maybe Python itself should have that warning/error?]
>>
>> Does that make sense?
>>
>
> I don't know if it is possible for Python to warn about this. I create an 
> issue on python-ideas.
>
> But your idea seems interesting (at least from a development point of 
> view).
>

https://mail.python.org/pipermail/python-ideas/2016-October/042782.html 

-- 
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/9fa02501-252f-474f-8956-6467cf7ecd35%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: is_valid as property

[Aymeric Augustin]

Hello Sven,

> On 11 Oct 2016, at 14:07, Sven R. Kunze  wrote:
> 
> I took a sample of 4 of my colleagues and asked them what the problem with 
> expressions like {% if form.is _valid %} is. They have no 
> idea. They said "it might not make sense in some cases but when it makes 
> sense, it doesn't look very terrible me”.

Can you show a non-contrived example, predating this discussion, of a situation 
where this pattern make sense?

> So, what's the real problem here?

The real problem is that templates aren’t the correct place to implement 
business logic such as form validation.

-- 
Aymeric.

-- 
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/D5E1BB5E-CB2D-4279-89D2-8852CEB9EAB6%40polytechnique.org.
For more options, visit https://groups.google.com/d/optout.

Re: is_valid as property

[Sven R. Kunze]

Am Donnerstag, 29. September 2016 21:25:44 UTC+2 schrieb Sjoerd Job Postmus:
>
> Thinking of an alternative route:
>
> It seems to me that the default `method.__bool__` is undesirable in Jinja2 
> templates. I do not know Jinja2 well enough, but maybe they could benefit 
> from a patch where `if`-statements give a warning/error when the expression 
> is a callable (with the default `FunctionType.__bool__`?
>
> This would solve the issue not just for the methods you mention, but more 
> in general.
>
> [Or maybe Python itself should have that warning/error?]
>
> Does that make sense?
>

I don't know if it is possible for Python to warn about this. I create an 
issue on python-ideas.

But your idea seems interesting (at least from a development point of view).

-- 
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/4386417c-fa74-4388-88e5-cb896b90f73a%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: is_valid as property

[Sven R. Kunze]

Am Donnerstag, 29. September 2016 22:05:43 UTC+2 schrieb Aymeric Augustin:
>
> On 29 Sep 2016, at 21:09, Sven R. Kunze  
> wrote:
>
> But independently, I still miss the point why {% if form.is_valid %} 
> cannot be a problem in Jinja2?
>
>
> If you’re writing that kind code in templates, you have much, much bigger 
> problems than what we’re discussing here!
>
>
I took a sample of 4 of my colleagues and asked them what the problem with 
expressions like {% if form.is_valid %} is. They have no idea. They said 
"it might not make sense in some cases but when it makes sense, it doesn't 
look very terrible me".

So, what's the real problem here?

Cheers,
Sven

PS: as said, we don't use it yet but we shouldn't it should be noted 
somewhere as it

-- 
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/f37da113-7fbc-4c94-9e23-69a42741a4ee%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: is_valid as property

[Michal Petrucha]

Hi everybody,

On Fri, Sep 30, 2016 at 09:04:32AM +0300, Shai Berger wrote:
> Hi Sven,
> 
> On Thursday 29 September 2016 22:09:27 Sven R. Kunze wrote:
> > 
> > Am Donnerstag, 29. September 2016 20:26:54 UTC+2 schrieb Aymeric Augustin:
> > 
> > > Generally speaking, properties are expected to be cheap and methods can be
> > > expensive. In my opinion, for lack of a better guideline, we should stick
> > > to this one. `is_valid()` falls into the expensive category, and for this
> > > reason it should remain a method.
> > 
> > Oh, I've never heard of this guideline. I just thought that properties
> > should be used to reduce visual noise (such as parentheses and "get_" or
> > "set_" prefixes.) or to replace a static attribute with a dynamic one.
> > Cheap vs. expensive never played a role so far (at least where I worked).
> 
> I'm completely with Aymeric on this one. The parentheses are not just noise, 
> they're a strong hint about the price of computation expected, and also about 
> the expectation of exceptions popping up. Properties hiding complex code are 
> downright misleading in my book, and "is_valid()", which may call user code, 
> certainly qualifies as complex.

I also agree with Aymeric and Shai on this, which is why I want to
bring up the elephant in the room: Form.errors. This is a property
which triggers full validation of the form on first access. What are
the chances of replacing Form.errors with a method?

Or maybe making it raise an error if it's accessed before the form has
been validated by calling an explicit method?

Cheers,

Michal

-- 
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/20160930094345.GY6601%40konk.org.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: Digital signature

Re: is_valid as property

[Shai Berger]

Hi Sven,

On Thursday 29 September 2016 22:09:27 Sven R. Kunze wrote:
> 
> Am Donnerstag, 29. September 2016 20:26:54 UTC+2 schrieb Aymeric Augustin:
> 
> > Generally speaking, properties are expected to be cheap and methods can be
> > expensive. In my opinion, for lack of a better guideline, we should stick
> > to this one. `is_valid()` falls into the expensive category, and for this
> > reason it should remain a method.
> 
> Oh, I've never heard of this guideline. I just thought that properties
> should be used to reduce visual noise (such as parentheses and "get_" or
> "set_" prefixes.) or to replace a static attribute with a dynamic one.
> Cheap vs. expensive never played a role so far (at least where I worked).

I'm completely with Aymeric on this one. The parentheses are not just noise, 
they're a strong hint about the price of computation expected, and also about 
the expectation of exceptions popping up. Properties hiding complex code are 
downright misleading in my book, and "is_valid()", which may call user code, 
certainly qualifies as complex.

Have fun,
Shai.

Re: is_valid as property

[Aymeric Augustin]

On 29 Sep 2016, at 21:09, Sven R. Kunze  wrote:

> But independently, I still miss the point why {% if form.is 
> _valid %} cannot be a problem in Jinja2?


If you’re writing that kind code in templates, you have much, much bigger 
problems than what we’re discussing here!

-- 
Aymeric.

-- 
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/0BE6327A-4446-47BD-8E8C-5CCFBAECC251%40polytechnique.org.
For more options, visit https://groups.google.com/d/optout.

Re: is_valid as property

[Sjoerd Job Postmus]

Thinking of an alternative route:
It seems to me that the default `method.__bool__` is undesirable in Jinja2 templates. I do not know Jinja2 well enough, but maybe they could benefit from a patch where `if`-statements give a warning/error when the _expression_ is a callable (with the default `FunctionType.__bool__`?
This would solve the issue not just for the methods you mention, but more in general.
[Or maybe Python itself should have that warning/error?]
Does that make sense?
On Sep 29, 2016 9:09 PM, "Sven R. Kunze"  wrote:Hi Aymeric,thanks for your ideas.Am Donnerstag, 29. September 2016 20:26:54 UTC+2 schrieb Aymeric Augustin:Hello,On 29 Sep 2016, at 19:57, Sven R. Kunze  wrote:#3 "Errors should never pass silently."Which they do if you write:if form.is_valid:    # will always be executed    # as it is always trueThis is perhaps the strongest argument in this discussion but I’m not convinced it’s strong enough to make the change.It’s weaker than the inconsistency that appeared between `{% if request.user.is_authenticated %}` in Django templates and `{% if request.user.is_authenticated() %}` in Jinja2 templates when Django started supporting both natively. The root cause of that inconsistency was Django’s auto-calling of callable in templates. This factor doesn’t exist here.That could depend on the perspective which argument is stronger or weaker. But independently, I still miss the point why {% if form.is_valid %} cannot be a problem in Jinja2? Writing `if some_callable:` instead of `if some_callable()` will cause the issue described here for any callable whose result makes sense in a boolean context. It’s always possible to build a security vulnerabilities with this behavior, by putting something security sensitive in the `if` or `else` block.Given that virtually anything can be evaluated in a boolean context in Python and in other dynamic languages such as _javascript_, I don’t know how to draw a line between what should be a property and what should be a method. For example, I don’t think we want to make QuerySet.exists a magic CallableBoolean, do we?Now, that you mentioned it  *seeking the "new topic" button* ;)Basically, the same arguments would apply there as well.Generally speaking, properties are expected to be cheap and methods can be expensive. In my opinion, for lack of a better guideline, we should stick to this one. `is_valid()` falls into the expensive category, and for this reason it should remain a method.Oh, I've never heard of this guideline. I just thought that properties should be used to reduce visual noise (such as parentheses and "get_" or "set_" prefixes.) or to replace a static attribute with a dynamic one. Cheap vs. expensive never played a role so far (at least where I worked). If that's relevant, we use "cached_property".Regards,Sven



-- 
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+unsubscribe@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/ecd30665-f698-4199-8fc3-d75ca66557d1%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 https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/88a914b2-13a0-40b8-bb42-20220977c13d%40email.android.com.
For more options, visit https://groups.google.com/d/optout.

Re: is_valid as property

[Sven R. Kunze]

Hi Aymeric,

thanks for your ideas.

Am Donnerstag, 29. September 2016 20:26:54 UTC+2 schrieb Aymeric Augustin:
>
> Hello,
>
> On 29 Sep 2016, at 19:57, Sven R. Kunze  
> wrote:
>
> #3 "Errors should never pass silently."
>
> Which they do if you write:
>
> if form.is_valid:
> # will always be executed
> # as it is always true
>
>
> This is perhaps the strongest argument in this discussion but I’m not 
> convinced it’s strong enough to make the change.
>
> It’s weaker than the inconsistency that appeared between `{% if 
> request.user.is_authenticated %}` in Django templates and `{% if 
> request.user.is_authenticated() %}` in Jinja2 templates when Django 
> started supporting both natively. The root cause of that inconsistency was 
> Django’s auto-calling of callable in templates. This factor doesn’t exist 
> here.
>

That could depend on the perspective which argument is stronger or weaker. 
But independently, I still miss the point why {% if form.is_valid %} cannot 
be a problem in Jinja2?
 

>
> Writing `if some_callable:` instead of `if some_callable()` will cause the 
> issue described here for any callable whose result makes sense in a boolean 
> context. It’s always possible to build a security vulnerabilities with this 
> behavior, by putting something security sensitive in the `if` or `else` 
> block.
>
> Given that virtually anything can be evaluated in a boolean context in 
> Python and in other dynamic languages such as JavaScript, I don’t know how 
> to draw a line between what should be a property and what should be a 
> method. For example, I don’t think we want to make QuerySet.exists a magic 
> CallableBoolean, do we?
>

Now, that you mentioned it  *seeking the "new topic" button* ;)

Basically, the same arguments would apply there as well.

Generally speaking, properties are expected to be cheap and methods can be 
> expensive. In my opinion, for lack of a better guideline, we should stick 
> to this one. `is_valid()` falls into the expensive category, and for this 
> reason it should remain a method.
>

Oh, I've never heard of this guideline. I just thought that properties 
should be used to reduce visual noise (such as parentheses and "get_" or 
"set_" prefixes.) or to replace a static attribute with a dynamic one. 
Cheap vs. expensive never played a role so far (at least where I worked). 
If that's relevant, we use "cached_property".

Regards,
Sven

-- 
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/ecd30665-f698-4199-8fc3-d75ca66557d1%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: is_valid as property

[Aymeric Augustin]

On 29 Sep 2016, at 20:26, Sjoerd Job Postmus  wrote:

> Replacing functions with properties will create a lot of churn in the people 
> using the mentioned functions.
> 
> Another approach might be to wrap these functions in a class with `__call__` 
> and `__bool__` both calling the underlying function. Or `__bool__` raising an 
> error? [1]

That’s how the change was handled for is_authenticated/anonymous. However 
(IIRC) the plan is to only support property access after the deprecation 
completes.

I’m afraid that supporting both styles in the long run would be confusing.

-- 
Aymeric.

-- 
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/EF103864-1ED3-4B9A-83A6-57048BE8A35C%40polytechnique.org.
For more options, visit https://groups.google.com/d/optout.

Re: is_valid as property

[Sven R. Kunze]

Am Donnerstag, 29. September 2016 18:55:37 UTC+2 schrieb Tim Graham:
>
> #1. Regarding templates, one of the arguments for the previous change was:
>
> Django 1.8 worsens the situation significantly:
>
> {% if request.user.is_authenticated %}
>
> does the right thing in a Django template but is a security vulnerability 
> in a Jinja2 template!
>


Ah, yes. I for one think that this is just a minor issue as changing the 
underlying template engine is not done on a regular basis. But that might 
just be me.

If it contributes as an argument, why not. I don't see anything wrong with 
using is_valid/has_changed in templates especially when we need to display 
field errors.

 

> #2. There was an inconsistency with the is_staff, is_active, is_superuser 
> attributes and is_anonymous, is_authenticated as methods. I'm not sure what 
> the inconsistency is with forms. Yes, there's an Form.is_bound property and 
> an is_multipart() method but I don't see a need to convert all is_*() 
> methods to properties.
>
>
> In my mind, the security ramifications were the main reason for the 
> previous change, and I don't see those concerns here. Changing 
> Form.is_valid() to a property seems like it would cause much more 
> disruption across the Django ecosystem than the gain would be worth.
>


It's always the combination of several issues. Here are mine:

#1 Inconsistency
For newbies that's the hardest part; especially when now compared to 
is_authenticated and other boolean attributes/properties even within Django.


#2 Wasted Effort
Also on a regular basis; which is quite problematic for commercial 
projects. Focusing on the topic at hand is more important than such 
technical details.
The mentioned dev who missed the "()" is one of our most experienced devs 
and that mentioned piece of code is only 3 years old.


#3 "Errors should never pass silently."

Which they do if you write:

if form.is_valid:
# will always be executed
# as it is always true


#4 Django devs are not only Django devs

They are Python devs as well and usually Django as a single lib is not 
enough. Especially Python devs expect things to work the same also across 
most libs such as boolean attributes/properties.


You might not find them worth enough to warrant the trouble. I for one do, 
hence the post. But it's not me to decide. :)


Some related thought: relating Django's past lifetime with its projected 
future lifetime which still lies ahead, Django is still in its infancy. 
So, I guess that any change no matter how "disruptive" for the sake of 
consistency will eventually pay out. Latter generations will be grateful 
not to distinguish between situations where they need "()" and where they 
don't.

-- 
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/e36d0f0a-fee8-4340-9d8a-24402dd55b76%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: is_valid as property

[Tim Graham]

#1. Regarding templates, one of the arguments for the previous change was:

Django 1.8 worsens the situation significantly:

{% if request.user.is_authenticated %}

does the right thing in a Django template but is a security vulnerability 
in a Jinja2 template!

#2. There was an inconsistency with the is_staff, is_active, is_superuser 
attributes and is_anonymous, is_authenticated as methods. I'm not sure what 
the inconsistency is with forms. Yes, there's an Form.is_bound property and 
an is_multipart() method but I don't see a need to convert all is_*() 
methods to properties.


In my mind, the security ramifications were the main reason for the 
previous change, and I don't see those concerns here. Changing 
Form.is_valid() to a property seems like it would cause much more 
disruption across the Django ecosystem than the gain would be worth.

On Thursday, September 29, 2016 at 11:52:55 AM UTC-4, Sven R. Kunze wrote:
>
> Am Donnerstag, 29. September 2016 15:52:00 UTC+2 schrieb Tim Graham:
>>
>> I don't think the same argument applies because no one (I hope) is 
>> calling form.is_valid() in templates.
>>
>
> Could you elaborate why this only plays a role with templates?
>
>
> So, as it turns out I ran grep on our code and easily found a place where 
> the programmer missed the "()" after is_valid. Regarding templates, I found 
> no occurrences of "is_valid" nor "has_changed" in our templates so far but 
> this might have other reasons as we don't have a very long history of 
> templates until recently. Additionally, what's so wrong about using either 
> function in templates?
>

-- 
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/f544f62b-7129-4991-bfaf-ff51926358be%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: is_valid as property

[Sven R. Kunze]

Am Donnerstag, 29. September 2016 15:52:00 UTC+2 schrieb Tim Graham:
>
> I don't think the same argument applies because no one (I hope) is calling 
> form.is_valid() in templates.
>

Could you elaborate why this only plays a role with templates?


So, as it turns out I ran grep on our code and easily found a place where 
the programmer missed the "()" after is_valid. Regarding templates, I found 
no occurrences of "is_valid" nor "has_changed" in our templates so far but 
this might have other reasons as we don't have a very long history of 
templates until recently. Additionally, what's so wrong about using either 
function in templates?

-- 
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/74aee2cb-66d7-466a-8a79-c0e25d91a95e%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: is_valid as property

[Tim Graham]

I don't think the same argument applies because no one (I hope) is calling 
form.is_valid() in templates.

On Thursday, September 29, 2016 at 9:38:07 AM UTC-4, Sven R. Kunze wrote:
>
> Good afternoon,
>
> I would like to follow up on 
> https://groups.google.com/forum/#!searchin/django-developers/is_authenticated$20as$20property%7Csort:relevance/django-developers/7k6Z8JxKH5Q/RoNKv66xcAMJ
>  
> with regards to Form.is_valid.
>
> It's is not as security relevant as is_authenticated but the remaining 
> arguments of "is_authenticated as property" still hold. I also suggest the 
> usage of CallableBool as a good temporary backwards-compatibility measure.
>
> Best
> Sven
>

-- 
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/e9a33d42-bc0d-4375-97ae-05231c385f17%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

is_valid as property

[Sven R. Kunze]

Good afternoon,

I would like to follow up on 
https://groups.google.com/forum/#!searchin/django-developers/is_authenticated$20as$20property%7Csort:relevance/django-developers/7k6Z8JxKH5Q/RoNKv66xcAMJ
 
with regards to Form.is_valid.

It's is not as security relevant as is_authenticated but the remaining 
arguments of "is_authenticated as property" still hold. I also suggest the 
usage of CallableBool as a good temporary backwards-compatibility measure.

Best
Sven

-- 
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/0b24ebd4-af4b-4a4c-8896-ea931411a06e%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.