Re: Ticket #25236: Remove ifequal from the template language

[Curtis Maloney]

I'd probably go with updating the documentation to say they're legacy
tags, you're better off using {% if %} now, and warn they may be
removed in a later release.

This way (a) people are warned off it, and (b) we're confusing people
who find it and want to check what it does [even though it's quite
clear from the name]

--
Curtis


On 7 August 2015 at 13:05, Josh Smeaton  wrote:
> Fair enough, I don't necessarily disagree with you. Would removing the
> documentation for ifequal be OK? At least that would prevent new projects
> from using it or being confused about multiple ways to compare values.
>
> On Friday, 7 August 2015 12:49:37 UTC+10, Karen Tracey wrote:
>>
>> We certainly weren't discussing removing without deprecating, were we? I'm
>> saying removing isn't worth the hassle to users, period. My opinion from
>> working on a fair number of inherited sites, plus sites where I don't get to
>> choose who contributes code I'm responsible for maintaining. I've reported I
>> have code I have to maintain written last year that uses these tags, it's
>> not just ancient crufty old stuff. These tags don't cause a massive "brain
>> stop" to figure out what "ifequal" might mean when encountering them in
>> existing templates.They are not that bad. This wart isn't worth the pain to
>> users to remove, in my opinion. Finding and fixing all templates used by a
>> site is NOT trivial. Fixing url was worth it. This is not.
>>
>>
> --
> 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/44ee5d1f-1071-4dd7-a1dd-fe798c0dc2de%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/CAG_XiSBPO35aTAemeP2%2BtFs2v-dacUHR3NiYoOFC8%3D4sYmgXhg%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Re: Ticket #25236: Remove ifequal from the template language

[Josh Smeaton]

Fair enough, I don't necessarily disagree with you. Would removing the 
documentation for ifequal be OK? At least that would prevent new projects 
from using it or being confused about multiple ways to compare values.

On Friday, 7 August 2015 12:49:37 UTC+10, Karen Tracey wrote:
>
> We certainly weren't discussing removing without deprecating, were we? I'm 
> saying removing isn't worth the hassle to users, period. My opinion from 
> working on a fair number of inherited sites, plus sites where I don't get 
> to choose who contributes code I'm responsible for maintaining. I've 
> reported I have code I have to maintain written last year that uses these 
> tags, it's not just ancient crufty old stuff. These tags don't cause a 
> massive "brain stop" to figure out what "ifequal" might mean when 
> encountering them in existing templates.They are not that bad. This wart 
> isn't worth the pain to users to remove, in my opinion. Finding and fixing 
> all templates used by a site is NOT trivial. Fixing url was worth it. This 
> is not.
>
>
>

-- 
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/44ee5d1f-1071-4dd7-a1dd-fe798c0dc2de%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Ticket #25236: Remove ifequal from the template language

[Karen Tracey]

We certainly weren't discussing removing without deprecating, were we? I'm
saying removing isn't worth the hassle to users, period. My opinion from
working on a fair number of inherited sites, plus sites where I don't get
to choose who contributes code I'm responsible for maintaining. I've
reported I have code I have to maintain written last year that uses these
tags, it's not just ancient crufty old stuff. These tags don't cause a
massive "brain stop" to figure out what "ifequal" might mean when
encountering them in existing templates.They are not that bad. This wart
isn't worth the pain to users to remove, in my opinion. Finding and fixing
all templates used by a site is NOT trivial. Fixing url was worth it. This
is not.

-- 
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/CACS9radxTh4mNtkQ%3Dfg5yUg_MF_EUu6Yqo%3DNGNqJg7e5vAwbwQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Re: Ticket #25236: Remove ifequal from the template language

[Josh Smeaton]

Can we just push the ifequal type tags through regular deprecation? As 
mentioned, the cost of maintaining the tags is extremely low. Remove the 
ifequal tags from the docs, document their deprecation, and remove in 
Django 2.0.

On Friday, 7 August 2015 11:56:29 UTC+10, Curtis Maloney wrote:
>
> Given the [currently] low burden, I agree with a more gradual 
> deprecation... document against their use to help encourage people to 
> move away from them, then we're in a better position to more 
> painlessly remove them in the future. 
>
> -- 
> Curtis 
>
> On 7 August 2015 at 11:43, Karen Tracey  
> wrote: 
> > Believe me I understand what technical debt is. In my opinion the cost 
> of 
> > this debt in Django is not sufficient to warrant the cost to users of 
> Django 
> > to remove it. Find and fix (or figure out if it's necessary to fix) all 
> > templates (some of which may be coming from 3rd party packages)  used in 
> a 
> > site is not that trivial of a task for a large site with many 3rd party 
> > dependencies. By contrast documenting why these two tags exist is pretty 
> > painless. 
> > 
> > -- 
> > 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 http://groups.google.com/group/django-developers. 
> > To view this discussion on the web visit 
> > 
> https://groups.google.com/d/msgid/django-developers/CACS9rafSuot0xati8HLOe2QEV7QpNny7d4bZDnVGzwsiAgDP7A%40mail.gmail.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/c12b57b2-88fd-4f72-9f65-302645f19823%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Ticket #25236: Remove ifequal from the template language

[Curtis Maloney]

Given the [currently] low burden, I agree with a more gradual
deprecation... document against their use to help encourage people to
move away from them, then we're in a better position to more
painlessly remove them in the future.

--
Curtis

On 7 August 2015 at 11:43, Karen Tracey  wrote:
> Believe me I understand what technical debt is. In my opinion the cost of
> this debt in Django is not sufficient to warrant the cost to users of Django
> to remove it. Find and fix (or figure out if it's necessary to fix) all
> templates (some of which may be coming from 3rd party packages)  used in a
> site is not that trivial of a task for a large site with many 3rd party
> dependencies. By contrast documenting why these two tags exist is pretty
> painless.
>
> --
> 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/CACS9rafSuot0xati8HLOe2QEV7QpNny7d4bZDnVGzwsiAgDP7A%40mail.gmail.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/CAG_XiSCwn53hms4SBCR5bLyAf-Cwnqk1jgcOwmv-FNPSoR2R1Q%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Re: Keeping apps without migrations?

[Markus Holtermann]

Instead of rushing to a decision if and of how we continue to support
"apps w/o migrations" I would argue that, since Django's test suite does
infact create tables for apps w/o migrations, we could add a commandline
flag (e.g. --run-syncdb or whatever) to the migrate command and warn the
user if there are apps w/o migrations an the user doesn't supplied:
https://github.com/django/django/blob/master/django/core/management/commands/migrate.py#L136

/Markus

On Thu, Aug 06, 2015 at 05:26:50PM -0700, Andrew Godwin wrote:

Do we know exactly what "support for apps without migrations" would consist
of at this point? I have that half-done code for replicating syncdb with
the autodetector stuck onto the migration executor, but it's not especially
speedy and would need some work to make it a sensible speed before it goes
forward.

Alternately, we could modify the autodetector to have a "create only" mode
that's more efficient or just write a new generator that is quick because
it knows it's only making nonmigrated apps' migrations.

Andrew

On Thu, Aug 6, 2015 at 4:56 PM, Tim Graham  wrote:


With the alpha for 1.9 coming up in 6 weeks, we need to decide whether or
not to continue support for apps without migrations (currently in master no
tables are created for such apps (as the deprecation timeline says), but it
might be appropriate to add a warning or error message for this case if
things don't change).

Is anyone sufficiently interested in continuing support for apps without
migrations to commit to completing it by the alpha? If not, do you consider
it a release blocker that I should spend time on?

Related tickets:

https://code.djangoproject.com/ticket/25144 - No way to create tables for
apps without migrations
https://code.djangoproject.com/ticket/24919 - Add an option not to run
migrations when running tests
https://code.djangoproject.com/ticket/24481 - Improve sqlmigrate to be
more flexible and allow bypassing migrations on disk
https://code.djangoproject.com/ticket/24901 - makemigrations should
create empty migrations dir for any installed app without it
https://code.djangoproject.com/ticket/24588 - Improve handling apps
without migrations while running migrate command.

On Sunday, January 18, 2015 at 5:11:53 PM UTC-5, Markus Holtermann wrote:


Creating in-memory migrations for all apps that don't have migration
files seems to be an option to solve the dependency problem. This would
even allow apps without migrations to depend on those with migrations.

We have to consider though, that there are tens of apps and hundreds of
models in our own test suite, and generating all migrations during start
seems to be quite an expensive task. And I'm not even talking about the
migration optimizer which probably needs to get a lot smarter if we take
this road.

syncdb, which leaves a developer with a database scheme that cannot be
altered automatically, is something we should get rid of as soon as
possible, especially since Django has a out-of-the-box migration system.

/Markus

On Sun, Jan 18, 2015 at 01:50:14PM -0800, Andrew Godwin wrote:
>My main argument for removing them entirely was the dependency issues
>between apps with and without migrations. Having syncdb use SchemaEditor
is
>a big step and one I'm happy we've got to, but the only advantage of
>keeping syncdb is for the test suite and I'd rather we approach that
more
>as "migrations made and run at runtime as a special case" rather than
>"syncdb".
>
>If nothing else, I'd like to see the end-developer-facing parts, like
the
>syncdb command itself, gone.
>
>Andrew
>
>On Sun, Jan 18, 2015 at 1:43 PM, Claude Paroz 
wrote:
>
>> Tim recently did a fabulous job of removing deprecated code for the
>> future 1.9 on master. Thanks for that.
>>
>> However, one thing he removed was support for apps without migrations.
>>
>>
https://github.com/django/django/commit/7e8cf74dc74539f40f4cea53c1e8bba82791fcb6
>>
>> Considering that we have to keep internal support for Django's own
test
>> suite anyway, I wonder if we should remove that support at all for
>> "normal" projects. I think one of Andrew's motivation was not to have
to
>> keep two schema editing code bases. But now that the old syncdb also
>> uses the new schema editor, I think that this argument doesn't stand.
>>
>> So what about keeping support for apps without migrations in the
longer
>> term. Of course, the fact that those apps cannot depend on migrated
apps
>> limits its use, but I think that for quick prototyping and initial
>> developement, migrations could be more of a hindrance. Would you see
>> major drawbacks with keeping this support?
>>
>> Opinions welcome.
>>
>> Claude
>> --
>> www.2xlibre.net


--
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 

Re: Ticket #25236: Remove ifequal from the template language

[Karen Tracey]

Believe me I understand what technical debt is. In my opinion the cost of
this debt in Django is not sufficient to warrant the cost to users of
Django to remove it. Find and fix (or figure out if it's necessary to fix)
all templates (some of which may be coming from 3rd party packages)  used
in a site is not that trivial of a task for a large site with many 3rd
party dependencies. By contrast documenting why these two tags exist is
pretty painless.

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

Re: #25227 Add utility method `get_updated_model()` to `ModelForm`

[Curtis Maloney]

Just my $0.02...  I quite like the idea of adding update_instance and
commit, with save calling both...

To me, this provides a clean and obvious place to customise updating
the instance before saving... as well as a clear place to get an
updated instance without saving.

--
Curtis


On 7 August 2015 at 10:14, Tim Graham  wrote:
> That sounds like a more promising proposal in my mind.
>
> So in a view instead of:
>
> if form.is_valid():
>obj = form.save(commit=False)
>obj.author = request.user
>obj.save()
>form.save_m2m()
>
> it might look like:
>
> if form.is_valid():
>form.instance.author = request.user
>form.save()
>
> I'm not sure if this will work (or result in simpler code) for model
> formsets though as there is the interaction with deleted formsets to
> consider.
>
>
> On Thursday, August 6, 2015 at 7:41:54 PM UTC-4, Javier Candeira wrote:
>>
>> Hi, Tim,
>>
>> Thanks for taking the time to address my ticket and patch.
>>
>> At this point I'm aware that I might be doing this just to practice
>> writing well-formed contributions to Django. At the very least I'll have
>> learnt a lot about how Django works on the inside, both the code and the
>> project.
>>
>> However, other developers have agreed with me that the save(commit=False)
>> is an antipattern. My goal is to remove that from tutorials, not to add a
>> get_updated_model() method. This was only a means to an end.
>>
>> For that reason I still think it's worth for me to have one more go at the
>> problem, and see if I can propose a different solution.
>>
>>> If we reevaluate as to whether super().save(commit=False) is needed at
>>> all, I think the forms.errors check isn't of much value (you will only hit
>>> that if you have in error in your code where you forgot to call is_valid())
>>> and save_m2m() could possibly be regular method that doesn't need to be
>>> conditionally added. But really, I don't think the current situation is so
>>> bad that we need to churn this. It doesn't seem feasible for the commit
>>> keyword to be deprecated, so now we would two ways of doing something.
>>
>>
>> In that case, a possible approach could be to turn `save(commit=False)`
>> into a does-nothing path, and suggest that the new way of doing things is:
>>
>> class ThingForm(ModelForm):
>> def save(self, commit=True):
>> self.instance.author = request.user
>> if commit:
>> super(ThingForm, self).save()
>>
>> As a first step to eventually deprecating commit, since it doesn't do
>> anything useful except return the instance, and suggest that the recommended
>> idiom is:
>>
>> class ThingForm(ModelForm):
>> def save(self):
>> self.instance.author = request.user
>> super(ThingForm, self).save()
>>
>> Cheers,
>>
>> Javier
>>
>>>
>>>
>>> On Thursday, August 6, 2015 at 6:51:23 PM UTC-4, Javier Candeira wrote:

 On Friday, 7 August 2015 08:38:33 UTC+10, Tim Graham wrote:
>
> I took a look at the code, found a bug, and proposed some
> simplifications in https://github.com/django/django/pull/5111


 Thanks, will check.

> The simplifications help make it clearer what get_updated_model() would
> do in the usual case:
> 1. Throw an error if the form has any errors: "The model could not be
> changed because the data didn't validate."
> 2. Adds a save_m2m() method to the form.
>
> It doesn't actually update the instance at all, so I guess the proposal
> is a bit misleading (unless I missed something).


 Yeah, I thought it did, but what it changes is the form (adding the
 save_m2m method). Maybe change name to get_model()?

>
> p.s. You can give your code a "self-review" with the patch review
> checklist:
> https://docs.djangoproject.com/en/1.8/internals/contributing/writing-code/submitting-patches/#patch-review-checklist


 Thanks. I've noticed that github tells me my code doesn't pass flake8
 either. I've recently changed to Spacemacs and couldn't get flake to work
 with it yesterday. I'll try before I update the PR/Ticket and write here
 again.

 Cheers,

 Javier

>
>
>
> On Thursday, August 6, 2015 at 6:09:54 PM UTC-4, Javier Candeira wrote:
>>
>> On Friday, 7 August 2015 07:19:01 UTC+10, Marc Tamlyn wrote:
>>>
>>> I agree that this is an anitpattern. I'm not fond of
>>> `get_updated_instance()` as a name, I'm not sure what the correct 
>>> option is
>>> here. Arguably we should split save() into two parts -
>>> ModelForm.update_instance() and ModelForm.commit(). ModelForm.save() 
>>> simply
>>> calls these two functions, with the latter only if commit=True.
>>
>>
>> get_updated_instance(form, instance, *args) is the module-level
>> function, parallell to save_instance(form, instance, *args). The exposed
>> method I suggest is 

Re: Keeping apps without migrations?

[Andrew Godwin]

Do we know exactly what "support for apps without migrations" would consist
of at this point? I have that half-done code for replicating syncdb with
the autodetector stuck onto the migration executor, but it's not especially
speedy and would need some work to make it a sensible speed before it goes
forward.

Alternately, we could modify the autodetector to have a "create only" mode
that's more efficient or just write a new generator that is quick because
it knows it's only making nonmigrated apps' migrations.

Andrew

On Thu, Aug 6, 2015 at 4:56 PM, Tim Graham  wrote:

> With the alpha for 1.9 coming up in 6 weeks, we need to decide whether or
> not to continue support for apps without migrations (currently in master no
> tables are created for such apps (as the deprecation timeline says), but it
> might be appropriate to add a warning or error message for this case if
> things don't change).
>
> Is anyone sufficiently interested in continuing support for apps without
> migrations to commit to completing it by the alpha? If not, do you consider
> it a release blocker that I should spend time on?
>
> Related tickets:
>
> https://code.djangoproject.com/ticket/25144 - No way to create tables for
> apps without migrations
> https://code.djangoproject.com/ticket/24919 - Add an option not to run
> migrations when running tests
> https://code.djangoproject.com/ticket/24481 - Improve sqlmigrate to be
> more flexible and allow bypassing migrations on disk
> https://code.djangoproject.com/ticket/24901 - makemigrations should
> create empty migrations dir for any installed app without it
> https://code.djangoproject.com/ticket/24588 - Improve handling apps
> without migrations while running migrate command.
>
> On Sunday, January 18, 2015 at 5:11:53 PM UTC-5, Markus Holtermann wrote:
>>
>> Creating in-memory migrations for all apps that don't have migration
>> files seems to be an option to solve the dependency problem. This would
>> even allow apps without migrations to depend on those with migrations.
>>
>> We have to consider though, that there are tens of apps and hundreds of
>> models in our own test suite, and generating all migrations during start
>> seems to be quite an expensive task. And I'm not even talking about the
>> migration optimizer which probably needs to get a lot smarter if we take
>> this road.
>>
>> syncdb, which leaves a developer with a database scheme that cannot be
>> altered automatically, is something we should get rid of as soon as
>> possible, especially since Django has a out-of-the-box migration system.
>>
>> /Markus
>>
>> On Sun, Jan 18, 2015 at 01:50:14PM -0800, Andrew Godwin wrote:
>> >My main argument for removing them entirely was the dependency issues
>> >between apps with and without migrations. Having syncdb use SchemaEditor
>> is
>> >a big step and one I'm happy we've got to, but the only advantage of
>> >keeping syncdb is for the test suite and I'd rather we approach that
>> more
>> >as "migrations made and run at runtime as a special case" rather than
>> >"syncdb".
>> >
>> >If nothing else, I'd like to see the end-developer-facing parts, like
>> the
>> >syncdb command itself, gone.
>> >
>> >Andrew
>> >
>> >On Sun, Jan 18, 2015 at 1:43 PM, Claude Paroz 
>> wrote:
>> >
>> >> Tim recently did a fabulous job of removing deprecated code for the
>> >> future 1.9 on master. Thanks for that.
>> >>
>> >> However, one thing he removed was support for apps without migrations.
>> >>
>> >>
>> https://github.com/django/django/commit/7e8cf74dc74539f40f4cea53c1e8bba82791fcb6
>> >>
>> >> Considering that we have to keep internal support for Django's own
>> test
>> >> suite anyway, I wonder if we should remove that support at all for
>> >> "normal" projects. I think one of Andrew's motivation was not to have
>> to
>> >> keep two schema editing code bases. But now that the old syncdb also
>> >> uses the new schema editor, I think that this argument doesn't stand.
>> >>
>> >> So what about keeping support for apps without migrations in the
>> longer
>> >> term. Of course, the fact that those apps cannot depend on migrated
>> apps
>> >> limits its use, but I think that for quick prototyping and initial
>> >> developement, migrations could be more of a hindrance. Would you see
>> >> major drawbacks with keeping this support?
>> >>
>> >> Opinions welcome.
>> >>
>> >> Claude
>> >> --
>> >> www.2xlibre.net
>>
> --
> 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
> 

Re: Ticket #25236: Remove ifequal from the template language

[Curtis Maloney]

Let me open by saying I am in no way averse to the removal of these
two tags... and this reminds me I need to also re-work my "remove the
'x as y' argument syntax" patch again...

On 7 August 2015 at 07:08, Marc Tamlyn  wrote:
> However, as with all technical debt, it has a cost. It's additional code,
> tests, documentation to maintain, low as the burden is. There is now "more
> than one way to do it" - when as a new developer who has learned {% if x ==
> y %} I then stumbles across {% ifequal x y %}, which is better? Does it
> matter? Is one more efficient?

As a small point, and I have met people for whom it was beneficial,
ifequal/ifnotequal _are_ more efficient than smart-if... it's a tiny
margin, but they are, and inside an inner loop, it can make quite a
difference.

I'm not advocating we keep them, just putting the info out there.

Personally, I'll just add another yak to my quiver of optimising if
... perhaps putting my AST skills to good use :)

--
C

-- 
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/CAG_XiSA0fp2GeQ1-rf2NtxW8gQ1GYJ52mwT%3D_1pZwhdWnZ-_vg%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Re: #25227 Add utility method `get_updated_model()` to `ModelForm`

[Tim Graham]

That sounds like a more promising proposal in my mind.

So in a view instead of:

if form.is_valid():
   obj = form.save(commit=False)
   obj.author = request.user
   obj.save()
   form.save_m2m()

it might look like:

if form.is_valid():
   form.instance.author = request.user
   form.save()

I'm not sure if this will work (or result in simpler code) for model 
formsets though as there is the interaction with deleted formsets to 
consider.

On Thursday, August 6, 2015 at 7:41:54 PM UTC-4, Javier Candeira wrote:
>
> Hi, Tim, 
>
> Thanks for taking the time to address my ticket and patch.
>
> At this point I'm aware that I might be doing this just to practice 
> writing well-formed contributions to Django. At the very least I'll have 
> learnt a lot about how Django works on the inside, both the code and the 
> project.
>
> However, other developers have agreed with me that the save(commit=False) 
> is an antipattern. My goal is to remove that from tutorials, not to add a 
> get_updated_model() method. This was only a means to an end.
>
> For that reason I still think it's worth for me to have one more go at the 
> problem, and see if I can propose a different solution.
>
> If we reevaluate as to whether super().save(commit=False) is needed at 
>> all, I think the forms.errors check isn't of much value (you will only hit 
>> that if you have in error in your code where you forgot to call is_valid()) 
>> and save_m2m() could possibly be regular method that doesn't need to be 
>> conditionally added. But really, I don't think the current situation is so 
>> bad that we need to churn this. It doesn't seem feasible for the commit 
>> keyword to be deprecated, so now we would two ways of doing something.
>>
>
> In that case, a possible approach could be to turn `save(commit=False)` 
> into a does-nothing path, and suggest that the new way of doing things is:
>
> class ThingForm(ModelForm):
> def save(self, commit=True):
> self.instance.author = request.user
> if commit:
> super(ThingForm, self).save()
>
> As a first step to eventually deprecating commit, since it doesn't do 
> anything useful except return the instance, and suggest that the 
> recommended idiom is:
>
> class ThingForm(ModelForm):
> def save(self):
> self.instance.author = request.user
> super(ThingForm, self).save()
>
> Cheers,
>
> Javier
>  
>
>>
>> On Thursday, August 6, 2015 at 6:51:23 PM UTC-4, Javier Candeira wrote:
>>>
>>> On Friday, 7 August 2015 08:38:33 UTC+10, Tim Graham wrote:

 I took a look at the code, found a bug, and proposed some 
 simplifications in https://github.com/django/django/pull/5111

>>>
>>> Thanks, will check. 
>>>
>>> The simplifications help make it clearer what get_updated_model() would 
 do in the usual case:
 1. Throw an error if the form has any errors: "The model could not be 
 changed because the data didn't validate."
 2. Adds a save_m2m() method to the form.

 It doesn't actually update the instance at all, so I guess the proposal 
 is a bit misleading (unless I missed something).

>>>
>>> Yeah, I thought it did, but what it changes is the form (adding the 
>>> save_m2m method). Maybe change name to get_model()?
>>>  
>>>
 p.s. You can give your code a "self-review" with the patch review 
 checklist: 
 https://docs.djangoproject.com/en/1.8/internals/contributing/writing-code/submitting-patches/#patch-review-checklist

>>>
>>> Thanks. I've noticed that github tells me my code doesn't pass flake8 
>>> either. I've recently changed to Spacemacs and couldn't get flake to work 
>>> with it yesterday. I'll try before I update the PR/Ticket and write here 
>>> again.
>>>
>>> Cheers,
>>>
>>> Javier
>>>  
>>>


 On Thursday, August 6, 2015 at 6:09:54 PM UTC-4, Javier Candeira wrote:
>
> On Friday, 7 August 2015 07:19:01 UTC+10, Marc Tamlyn wrote:
>>
>> I agree that this is an anitpattern. I'm not fond of 
>> `get_updated_instance()` as a name, I'm not sure what the correct option 
>> is 
>> here. Arguably we should split save() into two parts - 
>> ModelForm.update_instance() and ModelForm.commit(). ModelForm.save() 
>> simply 
>> calls these two functions, with the latter only if commit=True.
>>
>
> get_updated_instance(form, instance, *args) is the module-level 
> function, parallell to save_instance(form, instance, *args). The exposed 
> method I suggest is ModelForm.get_updated_model(self), which fits imho 
> because at this point we are coding in a domain where talking about forms 
> and models. 
>
> update_instance() could return the instance, but I'd prefer to suggest 
>> manipulating form.instance in the view.
>>
>
> This would be one(of many) more OO way of doing it, having 
> update_model() be a mutator method that doesn't return an instance, and 
> then accessing the instance 

Re: Keeping apps without migrations?

[Tim Graham]

With the alpha for 1.9 coming up in 6 weeks, we need to decide whether or 
not to continue support for apps without migrations (currently in master no 
tables are created for such apps (as the deprecation timeline says), but it 
might be appropriate to add a warning or error message for this case if 
things don't change).

Is anyone sufficiently interested in continuing support for apps without 
migrations to commit to completing it by the alpha? If not, do you consider 
it a release blocker that I should spend time on?

Related tickets:

https://code.djangoproject.com/ticket/25144 - No way to create tables for 
apps without migrations
https://code.djangoproject.com/ticket/24919 - Add an option not to run 
migrations when running tests
https://code.djangoproject.com/ticket/24481 - Improve sqlmigrate to be more 
flexible and allow bypassing migrations on disk
https://code.djangoproject.com/ticket/24901 - makemigrations should create 
empty migrations dir for any installed app without it
https://code.djangoproject.com/ticket/24588 - Improve handling apps without 
migrations while running migrate command.

On Sunday, January 18, 2015 at 5:11:53 PM UTC-5, Markus Holtermann wrote:
>
> Creating in-memory migrations for all apps that don't have migration 
> files seems to be an option to solve the dependency problem. This would 
> even allow apps without migrations to depend on those with migrations. 
>
> We have to consider though, that there are tens of apps and hundreds of 
> models in our own test suite, and generating all migrations during start 
> seems to be quite an expensive task. And I'm not even talking about the 
> migration optimizer which probably needs to get a lot smarter if we take 
> this road. 
>
> syncdb, which leaves a developer with a database scheme that cannot be 
> altered automatically, is something we should get rid of as soon as 
> possible, especially since Django has a out-of-the-box migration system. 
>
> /Markus 
>
> On Sun, Jan 18, 2015 at 01:50:14PM -0800, Andrew Godwin wrote: 
> >My main argument for removing them entirely was the dependency issues 
> >between apps with and without migrations. Having syncdb use SchemaEditor 
> is 
> >a big step and one I'm happy we've got to, but the only advantage of 
> >keeping syncdb is for the test suite and I'd rather we approach that more 
> >as "migrations made and run at runtime as a special case" rather than 
> >"syncdb". 
> > 
> >If nothing else, I'd like to see the end-developer-facing parts, like the 
> >syncdb command itself, gone. 
> > 
> >Andrew 
> > 
> >On Sun, Jan 18, 2015 at 1:43 PM, Claude Paroz  > wrote: 
> > 
> >> Tim recently did a fabulous job of removing deprecated code for the 
> >> future 1.9 on master. Thanks for that. 
> >> 
> >> However, one thing he removed was support for apps without migrations. 
> >> 
> >> 
> https://github.com/django/django/commit/7e8cf74dc74539f40f4cea53c1e8bba82791fcb6
>  
> >> 
> >> Considering that we have to keep internal support for Django's own test 
> >> suite anyway, I wonder if we should remove that support at all for 
> >> "normal" projects. I think one of Andrew's motivation was not to have 
> to 
> >> keep two schema editing code bases. But now that the old syncdb also 
> >> uses the new schema editor, I think that this argument doesn't stand. 
> >> 
> >> So what about keeping support for apps without migrations in the longer 
> >> term. Of course, the fact that those apps cannot depend on migrated 
> apps 
> >> limits its use, but I think that for quick prototyping and initial 
> >> developement, migrations could be more of a hindrance. Would you see 
> >> major drawbacks with keeping this support? 
> >> 
> >> Opinions welcome. 
> >> 
> >> Claude 
> >> -- 
> >> www.2xlibre.net 
>

-- 
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/89839628-6b11-4c61-86ed-4d9a076e9e3d%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: #25227 Add utility method `get_updated_model()` to `ModelForm`

[Javier Candeira]

Hi, Tim, 

Thanks for taking the time to address my ticket and patch.

At this point I'm aware that I might be doing this just to practice writing 
well-formed contributions to Django. At the very least I'll have learnt a 
lot about how Django works on the inside, both the code and the project.

However, other developers have agreed with me that the save(commit=False) 
is an antipattern. My goal is to remove that from tutorials, not to add a 
get_updated_model() method. This was only a means to an end.

For that reason I still think it's worth for me to have one more go at the 
problem, and see if I can propose a different solution.

If we reevaluate as to whether super().save(commit=False) is needed at all, 
> I think the forms.errors check isn't of much value (you will only hit that 
> if you have in error in your code where you forgot to call is_valid()) and 
> save_m2m() could possibly be regular method that doesn't need to be 
> conditionally added. But really, I don't think the current situation is so 
> bad that we need to churn this. It doesn't seem feasible for the commit 
> keyword to be deprecated, so now we would two ways of doing something.
>

In that case, a possible approach could be to turn `save(commit=False)` 
into a does-nothing path, and suggest that the new way of doing things is:

class ThingForm(ModelForm):
def save(self, commit=True):
self.instance.author = request.user
if commit:
super(ThingForm, self).save()

As a first step to eventually deprecating commit, since it doesn't do 
anything useful except return the instance, and suggest that the 
recommended idiom is:

class ThingForm(ModelForm):
def save(self):
self.instance.author = request.user
super(ThingForm, self).save()

Cheers,

Javier
 

>
> On Thursday, August 6, 2015 at 6:51:23 PM UTC-4, Javier Candeira wrote:
>>
>> On Friday, 7 August 2015 08:38:33 UTC+10, Tim Graham wrote:
>>>
>>> I took a look at the code, found a bug, and proposed some 
>>> simplifications in https://github.com/django/django/pull/5111
>>>
>>
>> Thanks, will check. 
>>
>> The simplifications help make it clearer what get_updated_model() would 
>>> do in the usual case:
>>> 1. Throw an error if the form has any errors: "The model could not be 
>>> changed because the data didn't validate."
>>> 2. Adds a save_m2m() method to the form.
>>>
>>> It doesn't actually update the instance at all, so I guess the proposal 
>>> is a bit misleading (unless I missed something).
>>>
>>
>> Yeah, I thought it did, but what it changes is the form (adding the 
>> save_m2m method). Maybe change name to get_model()?
>>  
>>
>>> p.s. You can give your code a "self-review" with the patch review 
>>> checklist: 
>>> https://docs.djangoproject.com/en/1.8/internals/contributing/writing-code/submitting-patches/#patch-review-checklist
>>>
>>
>> Thanks. I've noticed that github tells me my code doesn't pass flake8 
>> either. I've recently changed to Spacemacs and couldn't get flake to work 
>> with it yesterday. I'll try before I update the PR/Ticket and write here 
>> again.
>>
>> Cheers,
>>
>> Javier
>>  
>>
>>>
>>>
>>> On Thursday, August 6, 2015 at 6:09:54 PM UTC-4, Javier Candeira wrote:

 On Friday, 7 August 2015 07:19:01 UTC+10, Marc Tamlyn wrote:
>
> I agree that this is an anitpattern. I'm not fond of 
> `get_updated_instance()` as a name, I'm not sure what the correct option 
> is 
> here. Arguably we should split save() into two parts - 
> ModelForm.update_instance() and ModelForm.commit(). ModelForm.save() 
> simply 
> calls these two functions, with the latter only if commit=True.
>

 get_updated_instance(form, instance, *args) is the module-level 
 function, parallell to save_instance(form, instance, *args). The exposed 
 method I suggest is ModelForm.get_updated_model(self), which fits imho 
 because at this point we are coding in a domain where talking about forms 
 and models. 

 update_instance() could return the instance, but I'd prefer to suggest 
> manipulating form.instance in the view.
>

 This would be one(of many) more OO way of doing it, having 
 update_model() be a mutator method that doesn't return an instance, and 
 then accessing the instance through regular attribute access:

 class MyForm(ModelForm):
 def save():
 # this would return None:
 form.update_model()  
 # in case we're editing and not creating
 form.model.author = form.model.author if form.model.author else 
 request.user  
 form.model.last_edited_by = request.user
 super(MyForm, self).save()

 Note that I'm not advocating this, because I didn't want to make 
 breaking changes. I just wanted to make things clearer for beginners 
 without breaking backwards compatibility for existing users.

 Also, the above assumes the work 

Re: #25227 Add utility method `get_updated_model()` to `ModelForm`

[Tim Graham]

I don't think 'get_model()' makes it any more obvious about the two things 
that happen as compared to the name 'super().save(commit=False)'.

If we reevaluate as to whether super().save(commit=False) is needed at all, 
I think the forms.errors check isn't of much value (you will only hit that 
if you have in error in your code where you forgot to call is_valid()) and 
save_m2m() could possibly be regular method that doesn't need to be 
conditionally added. But really, I don't think the current situation is so 
bad that we need to churn this. It doesn't seem feasible for the commit 
keyword to be deprecated, so now we would two ways of doing something.

On Thursday, August 6, 2015 at 6:51:23 PM UTC-4, Javier Candeira wrote:
>
> On Friday, 7 August 2015 08:38:33 UTC+10, Tim Graham wrote:
>>
>> I took a look at the code, found a bug, and proposed some simplifications 
>> in https://github.com/django/django/pull/5111
>>
>
> Thanks, will check. 
>
> The simplifications help make it clearer what get_updated_model() would do 
>> in the usual case:
>> 1. Throw an error if the form has any errors: "The model could not be 
>> changed because the data didn't validate."
>> 2. Adds a save_m2m() method to the form.
>>
>> It doesn't actually update the instance at all, so I guess the proposal 
>> is a bit misleading (unless I missed something).
>>
>
> Yeah, I thought it did, but what it changes is the form (adding the 
> save_m2m method). Maybe change name to get_model()?
>  
>
>> p.s. You can give your code a "self-review" with the patch review 
>> checklist: 
>> https://docs.djangoproject.com/en/1.8/internals/contributing/writing-code/submitting-patches/#patch-review-checklist
>>
>
> Thanks. I've noticed that github tells me my code doesn't pass flake8 
> either. I've recently changed to Spacemacs and couldn't get flake to work 
> with it yesterday. I'll try before I update the PR/Ticket and write here 
> again.
>
> Cheers,
>
> Javier
>  
>
>>
>>
>> On Thursday, August 6, 2015 at 6:09:54 PM UTC-4, Javier Candeira wrote:
>>>
>>> On Friday, 7 August 2015 07:19:01 UTC+10, Marc Tamlyn wrote:

 I agree that this is an anitpattern. I'm not fond of 
 `get_updated_instance()` as a name, I'm not sure what the correct option 
 is 
 here. Arguably we should split save() into two parts - 
 ModelForm.update_instance() and ModelForm.commit(). ModelForm.save() 
 simply 
 calls these two functions, with the latter only if commit=True.

>>>
>>> get_updated_instance(form, instance, *args) is the module-level 
>>> function, parallell to save_instance(form, instance, *args). The exposed 
>>> method I suggest is ModelForm.get_updated_model(self), which fits imho 
>>> because at this point we are coding in a domain where talking about forms 
>>> and models. 
>>>
>>> update_instance() could return the instance, but I'd prefer to suggest 
 manipulating form.instance in the view.

>>>
>>> This would be one(of many) more OO way of doing it, having 
>>> update_model() be a mutator method that doesn't return an instance, and 
>>> then accessing the instance through regular attribute access:
>>>
>>> class MyForm(ModelForm):
>>> def save():
>>> # this would return None:
>>> form.update_model()  
>>> # in case we're editing and not creating
>>> form.model.author = form.model.author if form.model.author else 
>>> request.user  
>>> form.model.last_edited_by = request.user
>>> super(MyForm, self).save()
>>>
>>> Note that I'm not advocating this, because I didn't want to make 
>>> breaking changes. I just wanted to make things clearer for beginners 
>>> without breaking backwards compatibility for existing users.
>>>
>>> Also, the above assumes the work is still going to be done in the view. 
>>> Putting all the work in the form sounds better, as you say:
>>>
>>> One is to pass in an instance in creation of the form - 
 `PostForm(instance=Post(user=request.user))`.

>>>
>>> This looks like a poster case for a default value. In the field 
>>> definition, pass an argument "default = get_user_from_current_request" 
>>> where get_user_from_current_request() does what it says**, and done. But, 
>>> for other cases (or for overriding the default in this case), your second 
>>> option is better.
>>>
>>> The other, and my preferred option, is that the PostForm takes `user` as 
 a kwarg, and does this automatically in its save method. This encapsulates 
 the logic of the form completely, and also means you can add extra 
 validation at the form level. For example, you could check whether the 
 user 
 is an admin or not and allow admins to post longer messages than logged in 
 users, or include links.

>>>
>>> Agreed. Something like "if input would validate, take it, but if not, 
>>> put this instead". However, this would be a different ticket and a 
>>> different pull request.
>>>
>>> Since this is my first proposed contribution, 

Re: #25227 Add utility method `get_updated_model()` to `ModelForm`

[Javier Candeira]

What I said on the tracker for #25241:

Thanks for this. You're cleaning here a lot of of cruft that I left in my 
own #25227 because I didn't know whether it was used from other parts of 
Django.

I assume this [#25241] is as good as merged. I'll rebase my patch on your 
one before continuing with the #25227 discussion.

Cheers,

Javier

On Friday, 7 August 2015 08:38:33 UTC+10, Tim Graham wrote:
>
> I took a look at the code, found a bug, and proposed some simplifications 
> in https://github.com/django/django/pull/5111
>
> The simplifications help make it clearer what get_updated_model() would do 
> in the usual case:
> 1. Throw an error if the form has any errors: "The model could not be 
> changed because the data didn't validate."
> 2. Adds a save_m2m() method to the form.
>
> It doesn't actually update the instance at all, so I guess the proposal is 
> a bit misleading (unless I missed something).
>
> p.s. You can give your code a "self-review" with the patch review 
> checklist: 
> https://docs.djangoproject.com/en/1.8/internals/contributing/writing-code/submitting-patches/#patch-review-checklist
>
> On Thursday, August 6, 2015 at 6:09:54 PM UTC-4, Javier Candeira wrote:
>>
>> On Friday, 7 August 2015 07:19:01 UTC+10, Marc Tamlyn wrote:
>>>
>>> I agree that this is an anitpattern. I'm not fond of 
>>> `get_updated_instance()` as a name, I'm not sure what the correct option is 
>>> here. Arguably we should split save() into two parts - 
>>> ModelForm.update_instance() and ModelForm.commit(). ModelForm.save() simply 
>>> calls these two functions, with the latter only if commit=True.
>>>
>>
>> get_updated_instance(form, instance, *args) is the module-level function, 
>> parallell to save_instance(form, instance, *args). The exposed method I 
>> suggest is ModelForm.get_updated_model(self), which fits imho because at 
>> this point we are coding in a domain where talking about forms and models. 
>>
>> update_instance() could return the instance, but I'd prefer to suggest 
>>> manipulating form.instance in the view.
>>>
>>
>> This would be one(of many) more OO way of doing it, having update_model() 
>> be a mutator method that doesn't return an instance, and then accessing the 
>> instance through regular attribute access:
>>
>> class MyForm(ModelForm):
>> def save():
>> # this would return None:
>> form.update_model()  
>> # in case we're editing and not creating
>> form.model.author = form.model.author if form.model.author else 
>> request.user  
>> form.model.last_edited_by = request.user
>> super(MyForm, self).save()
>>
>> Note that I'm not advocating this, because I didn't want to make breaking 
>> changes. I just wanted to make things clearer for beginners without 
>> breaking backwards compatibility for existing users.
>>
>> Also, the above assumes the work is still going to be done in the view. 
>> Putting all the work in the form sounds better, as you say:
>>
>> One is to pass in an instance in creation of the form - 
>>> `PostForm(instance=Post(user=request.user))`.
>>>
>>
>> This looks like a poster case for a default value. In the field 
>> definition, pass an argument "default = get_user_from_current_request" 
>> where get_user_from_current_request() does what it says**, and done. But, 
>> for other cases (or for overriding the default in this case), your second 
>> option is better.
>>
>> The other, and my preferred option, is that the PostForm takes `user` as 
>>> a kwarg, and does this automatically in its save method. This encapsulates 
>>> the logic of the form completely, and also means you can add extra 
>>> validation at the form level. For example, you could check whether the user 
>>> is an admin or not and allow admins to post longer messages than logged in 
>>> users, or include links.
>>>
>>
>> Agreed. Something like "if input would validate, take it, but if not, put 
>> this instead". However, this would be a different ticket and a different 
>> pull request.
>>
>> Since this is my first proposed contribution, would you mind 
>> eyeball-linting my code and telling me what you think, independently of 
>> whether the PR gets accepted or not?
>>
>> Thanks again,
>>
>> Javier Candeira
>>  
>> ** Note: I haven't looked whether something like this is easy to do, but 
>> if it isn't, I feel another itch to scratch coming up.
>>
>>
>> On 6 August 2015 at 21:50, Javier Candeira  wrote:
>>>
 On Friday, 7 August 2015 01:57:46 UTC+10, Tim Graham wrote:
>
> Discouraging the use of super() seems like a bad idea that could limit 
> flexibility in the future. 
>

 Fair enough, but I'm not discouraging the use of super(). As I point 
 out in the ticket, the recommended pattern already ignores super() in the 
 commit=True path of the code, since it suggests this:

 class MyForm(ModelForm):
 def save(commit=True):
 model = super(MyForm, self).save(commit=False)

Re: #25227 Add utility method `get_updated_model()` to `ModelForm`

[Javier Candeira]

On Friday, 7 August 2015 08:38:33 UTC+10, Tim Graham wrote:
>
> I took a look at the code, found a bug, and proposed some simplifications 
> in https://github.com/django/django/pull/5111
>

Thanks, will check. 

The simplifications help make it clearer what get_updated_model() would do 
> in the usual case:
> 1. Throw an error if the form has any errors: "The model could not be 
> changed because the data didn't validate."
> 2. Adds a save_m2m() method to the form.
>
> It doesn't actually update the instance at all, so I guess the proposal is 
> a bit misleading (unless I missed something).
>

Yeah, I thought it did, but what it changes is the form (adding the 
save_m2m method). Maybe change name to get_model()?
 

> p.s. You can give your code a "self-review" with the patch review 
> checklist: 
> https://docs.djangoproject.com/en/1.8/internals/contributing/writing-code/submitting-patches/#patch-review-checklist
>

Thanks. I've noticed that github tells me my code doesn't pass flake8 
either. I've recently changed to Spacemacs and couldn't get flake to work 
with it yesterday. I'll try before I update the PR/Ticket and write here 
again.

Cheers,

Javier
 

>
>
> On Thursday, August 6, 2015 at 6:09:54 PM UTC-4, Javier Candeira wrote:
>>
>> On Friday, 7 August 2015 07:19:01 UTC+10, Marc Tamlyn wrote:
>>>
>>> I agree that this is an anitpattern. I'm not fond of 
>>> `get_updated_instance()` as a name, I'm not sure what the correct option is 
>>> here. Arguably we should split save() into two parts - 
>>> ModelForm.update_instance() and ModelForm.commit(). ModelForm.save() simply 
>>> calls these two functions, with the latter only if commit=True.
>>>
>>
>> get_updated_instance(form, instance, *args) is the module-level function, 
>> parallell to save_instance(form, instance, *args). The exposed method I 
>> suggest is ModelForm.get_updated_model(self), which fits imho because at 
>> this point we are coding in a domain where talking about forms and models. 
>>
>> update_instance() could return the instance, but I'd prefer to suggest 
>>> manipulating form.instance in the view.
>>>
>>
>> This would be one(of many) more OO way of doing it, having update_model() 
>> be a mutator method that doesn't return an instance, and then accessing the 
>> instance through regular attribute access:
>>
>> class MyForm(ModelForm):
>> def save():
>> # this would return None:
>> form.update_model()  
>> # in case we're editing and not creating
>> form.model.author = form.model.author if form.model.author else 
>> request.user  
>> form.model.last_edited_by = request.user
>> super(MyForm, self).save()
>>
>> Note that I'm not advocating this, because I didn't want to make breaking 
>> changes. I just wanted to make things clearer for beginners without 
>> breaking backwards compatibility for existing users.
>>
>> Also, the above assumes the work is still going to be done in the view. 
>> Putting all the work in the form sounds better, as you say:
>>
>> One is to pass in an instance in creation of the form - 
>>> `PostForm(instance=Post(user=request.user))`.
>>>
>>
>> This looks like a poster case for a default value. In the field 
>> definition, pass an argument "default = get_user_from_current_request" 
>> where get_user_from_current_request() does what it says**, and done. But, 
>> for other cases (or for overriding the default in this case), your second 
>> option is better.
>>
>> The other, and my preferred option, is that the PostForm takes `user` as 
>>> a kwarg, and does this automatically in its save method. This encapsulates 
>>> the logic of the form completely, and also means you can add extra 
>>> validation at the form level. For example, you could check whether the user 
>>> is an admin or not and allow admins to post longer messages than logged in 
>>> users, or include links.
>>>
>>
>> Agreed. Something like "if input would validate, take it, but if not, put 
>> this instead". However, this would be a different ticket and a different 
>> pull request.
>>
>> Since this is my first proposed contribution, would you mind 
>> eyeball-linting my code and telling me what you think, independently of 
>> whether the PR gets accepted or not?
>>
>> Thanks again,
>>
>> Javier Candeira
>>  
>> ** Note: I haven't looked whether something like this is easy to do, but 
>> if it isn't, I feel another itch to scratch coming up.
>>
>>
>> On 6 August 2015 at 21:50, Javier Candeira  wrote:
>>>
 On Friday, 7 August 2015 01:57:46 UTC+10, Tim Graham wrote:
>
> Discouraging the use of super() seems like a bad idea that could limit 
> flexibility in the future. 
>

 Fair enough, but I'm not discouraging the use of super(). As I point 
 out in the ticket, the recommended pattern already ignores super() in the 
 commit=True path of the code, since it suggests this:

 class MyForm(ModelForm):
 def save(commit=True):

Re: #25227 Add utility method `get_updated_model()` to `ModelForm`

[Tim Graham]

I took a look at the code, found a bug, and proposed some simplifications 
in https://github.com/django/django/pull/5111

The simplifications help make it clearer what get_updated_model() would do 
in the usual case:
1. Throw an error if the form has any errors: "The model could not be 
changed because the data didn't validate."
2. Adds a save_m2m() method to the form.

It doesn't actually update the instance at all, so I guess the proposal is 
a bit misleading (unless I missed something).

p.s. You can give your code a "self-review" with the patch review 
checklist: 
https://docs.djangoproject.com/en/1.8/internals/contributing/writing-code/submitting-patches/#patch-review-checklist

On Thursday, August 6, 2015 at 6:09:54 PM UTC-4, Javier Candeira wrote:
>
> On Friday, 7 August 2015 07:19:01 UTC+10, Marc Tamlyn wrote:
>>
>> I agree that this is an anitpattern. I'm not fond of 
>> `get_updated_instance()` as a name, I'm not sure what the correct option is 
>> here. Arguably we should split save() into two parts - 
>> ModelForm.update_instance() and ModelForm.commit(). ModelForm.save() simply 
>> calls these two functions, with the latter only if commit=True.
>>
>
> get_updated_instance(form, instance, *args) is the module-level function, 
> parallell to save_instance(form, instance, *args). The exposed method I 
> suggest is ModelForm.get_updated_model(self), which fits imho because at 
> this point we are coding in a domain where talking about forms and models. 
>
> update_instance() could return the instance, but I'd prefer to suggest 
>> manipulating form.instance in the view.
>>
>
> This would be one(of many) more OO way of doing it, having update_model() 
> be a mutator method that doesn't return an instance, and then accessing the 
> instance through regular attribute access:
>
> class MyForm(ModelForm):
> def save():
> # this would return None:
> form.update_model()  
> # in case we're editing and not creating
> form.model.author = form.model.author if form.model.author else 
> request.user  
> form.model.last_edited_by = request.user
> super(MyForm, self).save()
>
> Note that I'm not advocating this, because I didn't want to make breaking 
> changes. I just wanted to make things clearer for beginners without 
> breaking backwards compatibility for existing users.
>
> Also, the above assumes the work is still going to be done in the view. 
> Putting all the work in the form sounds better, as you say:
>
> One is to pass in an instance in creation of the form - 
>> `PostForm(instance=Post(user=request.user))`.
>>
>
> This looks like a poster case for a default value. In the field 
> definition, pass an argument "default = get_user_from_current_request" 
> where get_user_from_current_request() does what it says**, and done. But, 
> for other cases (or for overriding the default in this case), your second 
> option is better.
>
> The other, and my preferred option, is that the PostForm takes `user` as a 
>> kwarg, and does this automatically in its save method. This encapsulates 
>> the logic of the form completely, and also means you can add extra 
>> validation at the form level. For example, you could check whether the user 
>> is an admin or not and allow admins to post longer messages than logged in 
>> users, or include links.
>>
>
> Agreed. Something like "if input would validate, take it, but if not, put 
> this instead". However, this would be a different ticket and a different 
> pull request.
>
> Since this is my first proposed contribution, would you mind 
> eyeball-linting my code and telling me what you think, independently of 
> whether the PR gets accepted or not?
>
> Thanks again,
>
> Javier Candeira
>  
> ** Note: I haven't looked whether something like this is easy to do, but 
> if it isn't, I feel another itch to scratch coming up.
>
>
> On 6 August 2015 at 21:50, Javier Candeira  wrote:
>>
>>> On Friday, 7 August 2015 01:57:46 UTC+10, Tim Graham wrote:

 Discouraging the use of super() seems like a bad idea that could limit 
 flexibility in the future. 

>>>
>>> Fair enough, but I'm not discouraging the use of super(). As I point out 
>>> in the ticket, the recommended pattern already ignores super() in the 
>>> commit=True path of the code, since it suggests this:
>>>
>>> class MyForm(ModelForm):
>>> def save(commit=True):
>>> model = super(MyForm, self).save(commit=False)
>>> # do stuff to model
>>> if commit:
>>> model.save()
>>> form.save_m2m()
>>>
>>> Instead of this:
>>>
>>> class MyForm(ModelForm):
>>> def save(commit=True):
>>> model = super(MyForm, self).save(commit=False)
>>> # do stuff to model
>>> if commit:
>>> super(MyForm, self).save()
>>>
>>> These two are equivalent, the second one would actually use super() for 
>>> saving and committing. But Django prefers not to.
>>>
>>> So if there is a 

Re: #25227 Add utility method `get_updated_model()` to `ModelForm`

[Javier Candeira]

On Friday, 7 August 2015 07:19:01 UTC+10, Marc Tamlyn wrote:
>
> I agree that this is an anitpattern. I'm not fond of 
> `get_updated_instance()` as a name, I'm not sure what the correct option is 
> here. Arguably we should split save() into two parts - 
> ModelForm.update_instance() and ModelForm.commit(). ModelForm.save() simply 
> calls these two functions, with the latter only if commit=True.
>

get_updated_instance(form, instance, *args) is the module-level function, 
parallell to save_instance(form, instance, *args). The exposed method I 
suggest is ModelForm.get_updated_model(self), which fits imho because at 
this point we are coding in a domain where talking about forms and models. 

update_instance() could return the instance, but I'd prefer to suggest 
> manipulating form.instance in the view.
>

This would be one(of many) more OO way of doing it, having update_model() 
be a mutator method that doesn't return an instance, and then accessing the 
instance through regular attribute access:

class MyForm(ModelForm):
def save():
# this would return None:
form.update_model()  
# in case we're editing and not creating
form.model.author = form.model.author if form.model.author else 
request.user  
form.model.last_edited_by = request.user
super(MyForm, self).save()

Note that I'm not advocating this, because I didn't want to make breaking 
changes. I just wanted to make things clearer for beginners without 
breaking backwards compatibility for existing users.

Also, the above assumes the work is still going to be done in the view. 
Putting all the work in the form sounds better, as you say:

One is to pass in an instance in creation of the form - 
> `PostForm(instance=Post(user=request.user))`.
>

This looks like a poster case for a default value. In the field definition, 
pass an argument "default = get_user_from_current_request" where 
get_user_from_current_request() does what it says**, and done. But, for 
other cases (or for overriding the default in this case), your second 
option is better.

The other, and my preferred option, is that the PostForm takes `user` as a 
> kwarg, and does this automatically in its save method. This encapsulates 
> the logic of the form completely, and also means you can add extra 
> validation at the form level. For example, you could check whether the user 
> is an admin or not and allow admins to post longer messages than logged in 
> users, or include links.
>

Agreed. Something like "if input would validate, take it, but if not, put 
this instead". However, this would be a different ticket and a different 
pull request.

Since this is my first proposed contribution, would you mind 
eyeball-linting my code and telling me what you think, independently of 
whether the PR gets accepted or not?

Thanks again,

Javier Candeira
 
** Note: I haven't looked whether something like this is easy to do, but if 
it isn't, I feel another itch to scratch coming up.


On 6 August 2015 at 21:50, Javier Candeira  
> wrote:
>
>> On Friday, 7 August 2015 01:57:46 UTC+10, Tim Graham wrote:
>>>
>>> Discouraging the use of super() seems like a bad idea that could limit 
>>> flexibility in the future. 
>>>
>>
>> Fair enough, but I'm not discouraging the use of super(). As I point out 
>> in the ticket, the recommended pattern already ignores super() in the 
>> commit=True path of the code, since it suggests this:
>>
>> class MyForm(ModelForm):
>> def save(commit=True):
>> model = super(MyForm, self).save(commit=False)
>> # do stuff to model
>> if commit:
>> model.save()
>> form.save_m2m()
>>
>> Instead of this:
>>
>> class MyForm(ModelForm):
>> def save(commit=True):
>> model = super(MyForm, self).save(commit=False)
>> # do stuff to model
>> if commit:
>> super(MyForm, self).save()
>>
>> These two are equivalent, the second one would actually use super() for 
>> saving and committing. But Django prefers not to.
>>
>> So if there is a reason for rejecting my patch, "discouraging use of 
>> super()" should hardly be it.
>>
>> I could also include a documentation patch recommending the use of 
>> super() for the commit=True path of save(), but I think practicality beats 
>> purity, Django seems to agree, and this is the better code:
>>
>> class MyForm(ModelForm):
>> def save(commit=True):
>> model = self.get_updated_model()
>> # do stuff to model
>> if commit:
>> model.save()
>> form.save_m2m() 
>>
>> > I think Django's documentation describes the behavior pretty well. 
>> Perhaps the Django Girls tutorial could be improved instead. I don't recall 
>> having trouble understanding how this worked when I learned Django and 
>> introducing a new way to duplicate functionality of a 10 year old pattern 
>> doesn't seem worth it to me.
>>
>> > 
>> 

Re: #25227 Add utility method `get_updated_model()` to `ModelForm`

[Marc Tamlyn]

I agree that this is an anitpattern. I'm not fond of
`get_updated_instance()` as a name, I'm not sure what the correct option is
here. Arguably we should split save() into two parts -
ModelForm.update_instance() and ModelForm.commit(). ModelForm.save() simply
calls these two functions, with the latter only if commit=True.

update_instance() could return the instance, but I'd prefer to suggest
manipulating form.instance in the view.

***

Really though, I'd like to see the Django (and Django girls) tutorials not
use this pattern at all - the pattern of moving this logic out of the form
and into the view is the true antipattern here. In the case of the example
in the Django girls tutorial, it's a canonical example of adding the user
to the saved instance. There are in fact two better ways to achieve this.

One is to pass in an instance in creation of the form -
`PostForm(instance=Post(user=request.user))`.

The other, and my preferred option, is that the PostForm takes `user` as a
kwarg, and does this automatically in its save method. This encapsulates
the logic of the form completely, and also means you can add extra
validation at the form level. For example, you could check whether the user
is an admin or not and allow admins to post longer messages than logged in
users, or include links.

On 6 August 2015 at 21:50, Javier Candeira  wrote:

> On Friday, 7 August 2015 01:57:46 UTC+10, Tim Graham wrote:
>>
>> Discouraging the use of super() seems like a bad idea that could limit
>> flexibility in the future.
>>
>
> Fair enough, but I'm not discouraging the use of super(). As I point out
> in the ticket, the recommended pattern already ignores super() in the
> commit=True path of the code, since it suggests this:
>
> class MyForm(ModelForm):
> def save(commit=True):
> model = super(MyForm, self).save(commit=False)
> # do stuff to model
> if commit:
> model.save()
> form.save_m2m()
>
> Instead of this:
>
> class MyForm(ModelForm):
> def save(commit=True):
> model = super(MyForm, self).save(commit=False)
> # do stuff to model
> if commit:
> super(MyForm, self).save()
>
> These two are equivalent, the second one would actually use super() for
> saving and committing. But Django prefers not to.
>
> So if there is a reason for rejecting my patch, "discouraging use of
> super()" should hardly be it.
>
> I could also include a documentation patch recommending the use of super()
> for the commit=True path of save(), but I think practicality beats purity,
> Django seems to agree, and this is the better code:
>
> class MyForm(ModelForm):
> def save(commit=True):
> model = self.get_updated_model()
> # do stuff to model
> if commit:
> model.save()
> form.save_m2m()
>
> > I think Django's documentation describes the behavior pretty well.
> Perhaps the Django Girls tutorial could be improved instead. I don't recall
> having trouble understanding how this worked when I learned Django and
> introducing a new way to duplicate functionality of a 10 year old pattern
> doesn't seem worth it to me.
>
> >
> https://docs.djangoproject.com/en/1.8/topics/forms/modelforms/#the-save-method
> 
>
> Django documentation is stellar. This means that the "save(save=False)"
> API antipattern is very well documented indeed. This doesn't make it less
> of an antipattern for beginners, however well we explain it or, indeed,
> however well us-who-already-understand-it already understand it. I had been
> programming for decades when I learnt Django. Many people following the
> tutorials haven't. In some cases, they haven't even been alive for one
> decade.
>
> I'd like to hear the opinion of people who teach newcomers to programming,
> but let me also point this out: in separating the commit=False and
> commit=True paths of ModelForm.save() into the two functions
> get_updated_instance() and save_instance(), this refactor also enhances the
> readability of the code to a prospective Django contributor diving into the
> source code for the first time.
>
> Thanks,
>
> Javier
>
>
>> On Thursday, August 6, 2015 at 7:34:54 AM UTC-4, Javier Candeira wrote:
>>>
>>> Hi, I'm the author of Ticket #25227 Add utility method
>>> `get_updated_model()` to `ModelForm` and its accompanying patch.
>>>
>>> Ticket: https://code.djangoproject.com/ticket/25227
>>>
>>> Patch: https://github.com/django/django/pull/5105
>>>
>>> Here's the writeup to save everyone a click:
>>>
>>> Add utility method get_updated_model() to ModelForm
>>>
>>> Additionally, add utility method get_updated_models() to FormSet
>>>
>>> Rationale:
>>>
>>> While doing the djangogirls tutorial, I noticed that
>>> ModelForm.save(commit=False) is given to newcomers as a reasonable way to
>>> acquire a form's populated model. This is an antipattern for several
>>> 

Re: Ticket #25236: Remove ifequal from the template language

[Marc Tamlyn]

ifequal is technical debt in Django. It's a legacy way of doing things
which we would not add now. Sure, we don't have to remove it, it isn't
blocking us from doing anything else and it isn't broken, and it doesn't
need much maintenance.

However, as with all technical debt, it has a cost. It's additional code,
tests, documentation to maintain, low as the burden is. There is now "more
than one way to do it" - when as a new developer who has learned {% if x ==
y %} I then stumbles across {% ifequal x y %}, which is better? Does it
matter? Is one more efficient? They don't know that one is legacy code from
days when {% if %} was more stupid.

We want to grow the Django community and make it easier to use Django. We
also want to make sure that "There should be one-- and preferably only one
--obvious way to do it". This is why we have done things like removing
patterns(), why we rewrote transactions, why django.migrations is not
South, and why removing features which could have been deprecated in 1.2 is
important. Changes always result in work for upgrading existing sites, but
this is a cost the community should bear to make progress.

The number of sites using ifequal is small (it's very old, and not
recommended in any tutorial for many years). The number of future sites
which should not be using it, and future users (and maintainers of old
projects) who shouldn't have to worry about two ways to do the same thing
is large. The upgrade path is easy - you can achieve it with sed.

Quoting the 1.2 release notes: "There’s really no reason to use {% ifequal
%} or {% ifnotequal %} anymore, unless you’re the nostalgic type."

On 6 August 2015 at 21:07, Karen Tracey  wrote:

> On Thu, Aug 6, 2015 at 12:15 PM, Daniel Greenfeld 
> wrote:
>
>> No modern project uses ifequal. No one recommends it. I argue it is
>> taking up valuable bytes in the project. Let's remove it.
>>
>
> I maintain (did not write) a project written last year that has ifequal
> and ifnotequal. Is it really necessary to make it harder to upgrade this
> project to a more recent Django?
>
> --
> 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/CACS9radErjkfJHC0wVvPJWK4HpzVQiAyj0s_2ykG57EAG1%2BqLA%40mail.gmail.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/CAMwjO1EG9%2BOAWiW9iAT%3DnCFKD4_jDeBLVX2cJNXP6CUL9_uafA%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Re: future of QuerySet.extra()?

[Adam Johnson]

I've seen *extra()* misused in cases where developers came from a 
background of writing raw SQL and then used it rather than figuring out the 
(often relatively simple) ORM way of doing it. This is then a big 
maintenance burden, and the harsher warning against its use is a good idea.

Also thanks for documenting *RawSQL*, didn't realize it was usable until 
now so I've been able to get rid of some *extra()* usage in django-mysql.

On Wednesday, August 5, 2015 at 1:09:30 AM UTC+1, Josh Smeaton wrote:
>
> You're right about that. You can aggregate over custom expressions. For 
> aggregating over enums you should be able to use case expressions
>
> On Wed, 5 Aug 2015 at 05:05 Anssi Kääriäinen  > wrote:
>
>> On Tuesday, August 4, 2015, Shai Berger  
>> wrote:
>>>
>>> The classic database aggregation examples involve aggregation over time: 
>>> Sum
>>> of sales per quarter (which is, itself, a function over date); average
>>> temparature per month; etc. All these require group-by clauses which name
>>> (expressions over) columns which are not FKs.
>>>
>>>
>> The following should work:
>>
>>
>>  
>> qs.annotate(quarter=Quarter('sold_date')).values('quarter').annotate(sum=Sum('amount'))
>>
>> I recall some changes to how the group by expression is generated for 
>> expressions. If I recall correctly the group by should have the expression 
>> itself now, not the base columns of the expression.
>>
>> There are many similar examples involving "enum columns" -- columns with a
>>> limited set of choices, where the choices are not instances of another 
>>> model.
>>
>>
>> I'm not sure of this one.
>>
>>  - Anssi
>>
>> -- 
>> You received this message because you are subscribed to a topic in the 
>> Google Groups "Django developers (Contributions to Django itself)" group.
>> To unsubscribe from this topic, visit 
>> https://groups.google.com/d/topic/django-developers/FojuU0syO8Y/unsubscribe
>> .
>> To unsubscribe from this group and all its topics, 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/CALMtK1FqTkdmnXNYvjZ8U_QF9%2BWzrMCb1DDFQU5WB4SiKPX06g%40mail.gmail.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/15f6552c-992f-4073-9802-0613ca498777%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: #25227 Add utility method `get_updated_model()` to `ModelForm`

[Javier Candeira]

On Friday, 7 August 2015 01:57:46 UTC+10, Tim Graham wrote:
>
> Discouraging the use of super() seems like a bad idea that could limit 
> flexibility in the future. 
>

Fair enough, but I'm not discouraging the use of super(). As I point out in 
the ticket, the recommended pattern already ignores super() in the 
commit=True path of the code, since it suggests this:

class MyForm(ModelForm):
def save(commit=True):
model = super(MyForm, self).save(commit=False)
# do stuff to model
if commit:
model.save()
form.save_m2m()

Instead of this:

class MyForm(ModelForm):
def save(commit=True):
model = super(MyForm, self).save(commit=False)
# do stuff to model
if commit:
super(MyForm, self).save()

These two are equivalent, the second one would actually use super() for 
saving and committing. But Django prefers not to.

So if there is a reason for rejecting my patch, "discouraging use of 
super()" should hardly be it.

I could also include a documentation patch recommending the use of super() 
for the commit=True path of save(), but I think practicality beats purity, 
Django seems to agree, and this is the better code:

class MyForm(ModelForm):
def save(commit=True):
model = self.get_updated_model()
# do stuff to model
if commit:
model.save()
form.save_m2m() 

> I think Django's documentation describes the behavior pretty well. 
Perhaps the Django Girls tutorial could be improved instead. I don't recall 
having trouble understanding how this worked when I learned Django and 
introducing a new way to duplicate functionality of a 10 year old pattern 
doesn't seem worth it to me.

> 
https://docs.djangoproject.com/en/1.8/topics/forms/modelforms/#the-save-method 


Django documentation is stellar. This means that the "save(save=False)" API 
antipattern is very well documented indeed. This doesn't make it less of an 
antipattern for beginners, however well we explain it or, indeed, however 
well us-who-already-understand-it already understand it. I had been 
programming for decades when I learnt Django. Many people following the 
tutorials haven't. In some cases, they haven't even been alive for one 
decade. 

I'd like to hear the opinion of people who teach newcomers to programming, 
but let me also point this out: in separating the commit=False and 
commit=True paths of ModelForm.save() into the two functions 
get_updated_instance() and save_instance(), this refactor also enhances the 
readability of the code to a prospective Django contributor diving into the 
source code for the first time.

Thanks,

Javier
 

> On Thursday, August 6, 2015 at 7:34:54 AM UTC-4, Javier Candeira wrote:
>>
>> Hi, I'm the author of Ticket #25227 Add utility method 
>> `get_updated_model()` to `ModelForm` and its accompanying patch.
>>
>> Ticket: https://code.djangoproject.com/ticket/25227
>>
>> Patch: https://github.com/django/django/pull/5105
>>
>> Here's the writeup to save everyone a click:
>>
>> Add utility method get_updated_model() to ModelForm
>>
>> Additionally, add utility method get_updated_models() to FormSet
>>
>> Rationale:
>>
>> While doing the djangogirls tutorial, I noticed that 
>> ModelForm.save(commit=False) is given to newcomers as a reasonable way to 
>> acquire a form's populated model. This is an antipattern for several 
>> reasons:
>>
>>   - It's counterintuitive. To a newcomer, it's the same as 
>> ``save(save=no)``. 
>>   - It's a leaky abstraction. Understanding it requires understanding 
>> that the save method does two things: a) prepare the model, and b) save it 
>> to the database.
>>   - It doesn't name its effect or return well.
>>
>> All these problems are addressed in the current patch. Changes:
>>
>>  - Implement ModelForm.get_updated_model()
>>  - Implement FormSet.get_updated_models()
>>  - Refactor ModelForm.save() and FormSet.save() to allow the above.
>>  - Both the tests and contrib.auth have been modified: any call to 
>> save(commit=False) for the purpose of obtaining a populated model has been 
>> replaced by get_updated_model(). Tests still pass, I'm confident it's a 
>> successful refactor.
>> - New tests have been added for the new methods in different contexts 
>> (from a ModelForm, from a FormSet, etc).
>>  - documentation has also been modified to showcase the new methods.
>>
>> Notes:
>>
>> Uses of ModelForm.save(commit=False) in the codebase have been left alone 
>> wherever it was used for its side effects and not for its return value.
>>
>> The Djangogirls tutorial has a PR that depends on the changes in the 
>> present one:
>>
>> https://github.com/DjangoGirls/tutorial/pull/450
>>
>>

-- 
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 

Re: Ticket #25236: Remove ifequal from the template language

[Karen Tracey]

On Thu, Aug 6, 2015 at 12:15 PM, Daniel Greenfeld  wrote:

> No modern project uses ifequal. No one recommends it. I argue it is taking
> up valuable bytes in the project. Let's remove it.
>

I maintain (did not write) a project written last year that has ifequal and
ifnotequal. Is it really necessary to make it harder to upgrade this
project to a more recent Django?

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

Re: Ticket #25236: Remove ifequal from the template language

[Tzu-ping Chung]

Also +1 for its removal. And ifnotequal, too, since nobody seems to have 
mentioned this yet.

I would imagine a simple regex find-and-replace very much enough to fix 
most problems this causes. And if someone *really* want those tags, it is 
also pretty trivial to just pull them into a third-party app. One line in 
INSTALLED_APPS and a {% load %} tag fix everything.


Daniel Greenfeld於 2015年8月7日星期五 UTC+8上午12時15分49秒寫道：
>
> No modern project uses ifequal. No one recommends it. I argue it is taking 
> up valuable bytes in the project. Let's remove it.
>
> Reference https://code.djangoproject.com/ticket/25236
>
> In there you'll see Tim Graham mentions that older Django projects may 
> push back on it, and suggests that a good medium ground would be to remove 
> it from the documentation.
>
> Any comments?
>
> Regards,
>
> Daniel Roy Greenfeld
>

-- 
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/e96ef778-9546-4137-a870-e4b833cdbef1%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Ticket #25236: Remove ifequal from the template language

[Marc Tamlyn]

I commented on the ticket - it's been redundant since Django 1.2 when the
smart if was introduced. +1 to deprecating it.

On 6 August 2015 at 17:15, Daniel Greenfeld  wrote:

> No modern project uses ifequal. No one recommends it. I argue it is taking
> up valuable bytes in the project. Let's remove it.
>
> Reference https://code.djangoproject.com/ticket/25236
>
> In there you'll see Tim Graham mentions that older Django projects may
> push back on it, and suggests that a good medium ground would be to remove
> it from the documentation.
>
> Any comments?
>
> Regards,
>
> Daniel Roy Greenfeld
>
> --
> 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/add63617-a253-4571-a705-bf5773ed18ec%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/CAMwjO1ECUQF7v6Mx33oOh-6JjY_12B8aw6sJ1Svv4-CXEuLGdw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Ticket #25236: Remove ifequal from the template language

[Daniel Greenfeld]

No modern project uses ifequal. No one recommends it. I argue it is taking 
up valuable bytes in the project. Let's remove it.

Reference https://code.djangoproject.com/ticket/25236

In there you'll see Tim Graham mentions that older Django projects may push 
back on it, and suggests that a good medium ground would be to remove it 
from the documentation.

Any comments?

Regards,

Daniel Roy Greenfeld

-- 
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/add63617-a253-4571-a705-bf5773ed18ec%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Help needed with Oracle GIS backend

[Tim Graham]

As we approach 1.9 alpha in about a month and half, no one has stepped up 
to offer help fix the Oracle GIS backend. Should we use another platform 
like the djangoproject.com blog to make a final plea for help?

On Monday, March 30, 2015 at 9:34:10 AM UTC-4, Jani Tiainen wrote:
>
> On Thu, 26 Mar 2015 22:29:00 +0200 
> Shai Berger  wrote: 
>
> > Hi Jani. 
> > 
> > On Wednesday 25 March 2015 09:58:00 Jani Tiainen wrote: 
> > > 
> > > We're still running Oracle and GIS. Though we do have built custom 
> > > backend based on Django GIS backend - mainly to have support for 
> > > Oracle XE, 3D functionality and in general to make it faster. 
> > > 
> > > It's currently closed source but we're working on to open source it 
> > > in hope that it could interest more wider audience (mainly because it 
> > > would enable GIS use with free Oracle XE). 
> > > 
> > That is good news. I would like to draw your attention to 
> > https://code.djangoproject.com/ticket/21273 where a partial attempt was 
> > already made to add GIS/Oracle-XE support. 
> > 
> > Thanks, 
> > Shai. 
>
> Well, last comment is mine :)  describing exactly how our backend does 
> work in general level and explains rationales why we chose a slightly 
> different approach. 
>
>
> -- 
> Jani Tiainen 
>

-- 
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/e8b754eb-771c-49e0-8b16-0ec24fa5099e%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: #25227 Add utility method `get_updated_model()` to `ModelForm`

[Tim Graham]

Discouraging the use of super() seems like a bad idea that could limit 
flexibility in the future. I think Django's documentation describes the 
behavior pretty well. Perhaps the Django Girls tutorial could be improved 
instead. I don't recall having trouble understanding how this worked when I 
learned Django and introducing a new way to duplicate functionality of a 10 
year old pattern doesn't seem worth it to me.

https://docs.djangoproject.com/en/1.8/topics/forms/modelforms/#the-save-method

On Thursday, August 6, 2015 at 7:34:54 AM UTC-4, Javier Candeira wrote:
>
> Hi, I'm the author of Ticket #25227 Add utility method 
> `get_updated_model()` to `ModelForm` and its accompanying patch.
>
> Ticket: https://code.djangoproject.com/ticket/25227
>
> Patch: https://github.com/django/django/pull/5105
>
> Here's the writeup to save everyone a click:
>
> Add utility method get_updated_model() to ModelForm
>
> Additionally, add utility method get_updated_models() to FormSet
>
> Rationale:
>
> While doing the djangogirls tutorial, I noticed that 
> ModelForm.save(commit=False) is given to newcomers as a reasonable way to 
> acquire a form's populated model. This is an antipattern for several 
> reasons:
>
>   - It's counterintuitive. To a newcomer, it's the same as 
> ``save(save=no)``. 
>   - It's a leaky abstraction. Understanding it requires understanding that 
> the save method does two things: a) prepare the model, and b) save it to 
> the database.
>   - It doesn't name its effect or return well.
>
> All these problems are addressed in the current patch. Changes:
>
>  - Implement ModelForm.get_updated_model()
>  - Implement FormSet.get_updated_models()
>  - Refactor ModelForm.save() and FormSet.save() to allow the above.
>  - Both the tests and contrib.auth have been modified: any call to 
> save(commit=False) for the purpose of obtaining a populated model has been 
> replaced by get_updated_model(). Tests still pass, I'm confident it's a 
> successful refactor.
> - New tests have been added for the new methods in different contexts 
> (from a ModelForm, from a FormSet, etc).
>  - documentation has also been modified to showcase the new methods.
>
> Notes:
>
> Uses of ModelForm.save(commit=False) in the codebase have been left alone 
> wherever it was used for its side effects and not for its return value.
>
> The Djangogirls tutorial has a PR that depends on the changes in the 
> present one:
>
> https://github.com/DjangoGirls/tutorial/pull/450
>
>

-- 
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/9c5eb1c5-ce13-48ea-8fe9-af013e5e1d45%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: How to disable system check framework?

[Tim Graham]

Perhaps your monkeypatches should be moved to `AppConfig.ready()` methods 
so they are applied before the system check framework is invoked? Model and 
admin validation were part of Django 1.4 too, by the way.

How many problematic checks are you running into? Can you describe them a 
bit more? If anything, SILENCED_SYSTEM_CHECKS = ['*'] to disable checks 
seems like the best option that Josh proposed, but I'd like more 
justification before we spend time doing that.

Does anyone else have a desire to completely disable system checks?

On Wednesday, August 5, 2015 at 5:09:07 AM UTC-4, Marcin Nowak wrote:
>
> Thank, Josh.
>
> I'm upgrading mature and complex project, which have some tricky parts 
> implemented like a monkey-patched user model, which is applied after system 
> chceck execution. So Django complains about missing fields in ModelAdmin, 
> but patch works fine. There were some complains about doubled mappings to 
> fields, but I've removed them. Generally speaking I don't need System 
> Check, because I can find soultion myself other way.
>
> Migrations are similar - they are too simple/limited for my needs. They 
> operate on models, but I'm working mainly with database and I'm creating 
> mappings for tables and views for easy querying. So I don't need builtin 
> migrations feature.
>
> I'm using commit_manually() frequently, but this decorator was removed... 
>
> And so on...
>
> That's why I feel that working with Django is harder. It's more strict, 
> useful tools/helpers are missing, there is more features which should be 
> bypassed/omitted. I feel that is harder to do simple things... Or maybe I'm 
> getting old. ;)
>
> Cheers,
> Marcin
>
>
> On Wednesday, August 5, 2015 at 3:08:26 AM UTC+2, Josh Smeaton wrote:
>>
>> Your "scare quotes" for "feature" are really disappointing. Especially 
>> considering that the checks were hardcoded and couldn't be silenced prior 
>> to the "system check framework".
>>
>> > Working with Django is getting harder with every new version.
>>
>> If you want to turn off new features or extension points, then I'd 
>> imagine there'd be more work than for the regular user. Otherwise, I 
>> couldn't disagree more.
>>
>> Anyhow, you want to turn off system checks for some reason. Here are some 
>> potential ways forward:
>>
>> - Target the checks that are incorrectly spitting out errors or warnings, 
>> and fix those.
>> - Provide another setting "DISABLE_CHECKS" or something similar that will 
>> prevent checks running altogether (probably not ideal, new settings are 
>> hard to get into core, especially when the benefit here is still 
>> questionable).
>> - Provide a special value for the existing SILENCED_SYSTEM_CHECKS setting 
>> that causes all checks to be silenced.
>>
>> You haven't really said much about the issue other than it bothers you. 
>> The main purpose of checks are for development - so it doesn't have any 
>> runtime or deployment time penalties. Perhaps if you took some time to 
>> justify why users other than yourself might want to disable the check 
>> framework, it'd be easier to start looking at solutions to problems.
>>
>> Cheers
>>
>>
>> On Wednesday, 5 August 2015 01:16:21 UTC+10, Marcin Nowak wrote:
>>>
>>> Thanks, Carl. I know about silencing but I want to disable unwanted 
>>> "feature". Working with Django is getting harder with every new version. 
>>>
>>>
>>> On Tuesday, August 4, 2015 at 3:25:22 PM UTC+2, Carl Meyer wrote:


 > On Aug 4, 2015, at 9:18 AM, Marcin Nowak  
 wrote: 
 > 
 > I need to upgrade project to Django 1.8 but the SystemCheck Framework 
 bothers me. 
 > It complains about thigs that should work, even if they are a little 
 tricky. 
 > 
 > I need to disable system checks designed for blogs and other simple 
 sites. How can I do that? 

 The third paragraph of the system checks documentation [1] links to 
 [2]. 

 Also, this is a question about using Django, not developing Django, so 
 it belongs on the django-users mailing list, not here. 

 Carl 

 [1] https://docs.djangoproject.com/en/1.8/topics/checks/ 
 [2] 
 https://docs.djangoproject.com/en/1.8/ref/settings/#std:setting-SILENCED_SYSTEM_CHECKS
>>>
>>>

-- 
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/4d5608dd-de84-476d-8934-bd59cb0fad66%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Decoration of class based views

[Tim Graham]

A few days ago, we added the ability to use @method_decorator at the class 
level:

https://github.com/django/django/commit/3bdaaf6777804d33ee46cdb5a889b8cc544a91f1

Does it help?

Is your proposal to add the reduce() syntax to the docs?

On Thursday, August 6, 2015 at 7:21:36 AM UTC-4, Fabrizio Messina wrote:
>
> Hello I would like to ask why the class based views documentation seems so 
> much ugly. Some developers probably are scared by these just because the 
> decoration is ugly, the documentation offers three ways:
>
> Decorate the *Klass().dispach()* method of the class, wrapping the 
> decorators in another decorator:
>
> @method_decorator(decorator_1)
> def dispatch(request, *args, **kwargs):
>   pass
>
> Wrapping the *Klass.as_view()* resulting in an arguably ugly
>
> *:*
>
>
> *decorator_1( decorator_2( .. ( .. decorator_n( Klass.as_view() ) .. ) .. 
> ))*In my opinion this is an issue because decoration is actually cool and the 
> alternative is to use the much more dreadful Multiple Inheritance, that is 
> the documented third way.
>
>
> I personally use reduce syntax:
>
> view = reduce(lambda x, f: f(x), (decorator_1, decorator_2, .., 
> decorator_n), Klass.as_view())
>
> Probably there are better ways but still to me it seems to me that 
> something like this is more readable, easier to modify and less scary. To 
> me it has the pro that one could easily wrap common used decorators in a 
> single iterable.
>

-- 
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/d59e9094-cf3c-47de-a132-bdc14429f31b%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Unable save model in admin with HStoreField

[Tim Graham]

There is no need to cross-post to the mailing list when you also created a 
ticket: https://code.djangoproject.com/ticket/25233

"Is this a bug?" questions should go to the django-users mailing list. 
Thanks!

On Thursday, August 6, 2015 at 7:21:36 AM UTC-4, Doan Hong Phi wrote:
>
> f = Food.objects.get(id=2)
> f.weight= {'0.5': '0.5kg - 1kg'}
> f.save()
>
> Save model by code then OK. But I can't save model in admin. See error 
> below:
>
>
> TypeError at /admin/food/food/2/
>
> expected string or buffer
>
> Request Method:POSTRequest URL:http://127.0.0.1:8000/admin/food/food/2/Django 
> Version:1.8.3Exception Type:TypeErrorException Value:
>
> expected string or buffer
>
> Exception Location:C:\Python27\Lib\json\decoder.py in decode, line 365Python 
> Executable:D:\Program Files\PythonEnv\foodgreen\Scripts\python.exePython 
> Version:2.7.6Python Path:
>
> ['D:\\Projects\\Django8\\greenfood',
>  'D:\\Program 
> Files\\eclipse\\plugins\\org.python.pydev_4.2.0.201507041133\\pysrc',
>  'D:\\Projects\\Django8\\greenfood',
>  'D:\\Program 
> Files\\PythonEnv\\foodgreen\\lib\\site-packages\\psycopg2-2.6.1-py2.7-win32.egg',
>  'D:\\Program Files\\PythonEnv\\foodgreen\\lib',
>  'D:\\Program Files\\PythonEnv\\foodgreen\\Scripts',
>  'C:\\Python27\\Lib',
>  'C:\\Python27\\DLLs',
>  'C:\\Python27\\Lib\\lib-tk',
>  'D:\\Program Files\\PythonEnv\\foodgreen',
>  'D:\\Program Files\\PythonEnv\\foodgreen\\lib\\site-packages',
>  'C:\\WINDOWS\\SYSTEM32\\python27.zip',
>  'D:\\Program Files\\PythonEnv\\foodgreen\\DLLs',
>  'D:\\Program Files\\PythonEnv\\foodgreen\\lib\\plat-win',
>  'D:\\Program Files\\PythonEnv\\foodgreen\\lib\\lib-tk']
>
> Server time:Thu, 6 Aug 2015 16:48:26 +0700
> Traceback Switch to copy-and-paste view 
> 
>
>- D:\Program 
>Files\PythonEnv\foodgreen\lib\site-packages\django\core\handlers\base.py
> in get_response
>1. 
>   
>   response = wrapped_callback(request, 
> *callback_args, **callback_kwargs)
>   
>   ...
>▶ Local vars 
>- D:\Program 
>Files\PythonEnv\foodgreen\lib\site-packages\django\contrib\admin\options.py
> in wrapper
>1. 
>   
>   return 
> self.admin_site.admin_view(view)(*args, **kwargs)
>   
>   ...
>▶ Local vars 
>- D:\Program 
>Files\PythonEnv\foodgreen\lib\site-packages\django\utils\decorators.py
> in _wrapped_view
>1. 
>   
>   response = view_func(request, *args, 
> **kwargs)
>   
>   ...
>▶ Local vars 
>- D:\Program 
>
> Files\PythonEnv\foodgreen\lib\site-packages\django\views\decorators\cache.py
> in _wrapped_view_func
>1. 
>   
>   response = view_func(request, *args, **kwargs)
>   
>   ...
>▶ Local vars 
>- D:\Program 
>Files\PythonEnv\foodgreen\lib\site-packages\django\contrib\admin\sites.py
> in inner
>1. 
>   
>   return view(request, *args, **kwargs)
>   
>   ...
>▶ Local vars 
>- D:\Program 
>Files\PythonEnv\foodgreen\lib\site-packages\django\contrib\admin\options.py
> in change_view
>1. 
>   
>   return self.changeform_view(request, object_id, 
> form_url, extra_context)
>   
>   ...
>▶ Local vars 
>- D:\Program 
>Files\PythonEnv\foodgreen\lib\site-packages\django\utils\decorators.py
> in _wrapper
>1. 
>   
>   return bound_func(*args, **kwargs)
>   
>   ...
>▶ Local vars 
>- D:\Program 
>Files\PythonEnv\foodgreen\lib\site-packages\django\utils\decorators.py
> in _wrapped_view
>1. 
>   
>   response = view_func(request, *args, 
> **kwargs)
>   
>   ...
>▶ Local vars 
>- D:\Program 
>Files\PythonEnv\foodgreen\lib\site-packages\django\utils\decorators.py
> in bound_func
>1. 
>   
>   return func.__get__(self, 
> type(self))(*args2, **kwargs2)
>   
>   ...
>▶ Local vars 
>- D:\Program 
>Files\PythonEnv\foodgreen\lib\site-packages\django\utils\decorators.py
> in inner
>1. 
>   
>   return func(*args, **kwargs)
>   
>   ...
>▶ Local vars 
>- D:\Program 
>Files\PythonEnv\foodgreen\lib\site-packages\django\contrib\admin\options.py
> in changeform_view
>1. 
>   
>  

#25227 Add utility method `get_updated_model()` to `ModelForm`

[Javier Candeira]

Hi, I'm the author of Ticket #25227 Add utility method 
`get_updated_model()` to `ModelForm` and its accompanying patch.

Ticket: https://code.djangoproject.com/ticket/25227

Patch: https://github.com/django/django/pull/5105

Here's the writeup to save everyone a click:

Add utility method get_updated_model() to ModelForm

Additionally, add utility method get_updated_models() to FormSet

Rationale:

While doing the djangogirls tutorial, I noticed that 
ModelForm.save(commit=False) is given to newcomers as a reasonable way to 
acquire a form's populated model. This is an antipattern for several 
reasons:

  - It's counterintuitive. To a newcomer, it's the same as 
``save(save=no)``. 
  - It's a leaky abstraction. Understanding it requires understanding that 
the save method does two things: a) prepare the model, and b) save it to 
the database.
  - It doesn't name its effect or return well.

All these problems are addressed in the current patch. Changes:

 - Implement ModelForm.get_updated_model()
 - Implement FormSet.get_updated_models()
 - Refactor ModelForm.save() and FormSet.save() to allow the above.
 - Both the tests and contrib.auth have been modified: any call to 
save(commit=False) for the purpose of obtaining a populated model has been 
replaced by get_updated_model(). Tests still pass, I'm confident it's a 
successful refactor.
- New tests have been added for the new methods in different contexts (from 
a ModelForm, from a FormSet, etc).
 - documentation has also been modified to showcase the new methods.

Notes:

Uses of ModelForm.save(commit=False) in the codebase have been left alone 
wherever it was used for its side effects and not for its return value.

The Djangogirls tutorial has a PR that depends on the changes in the 
present one:

https://github.com/DjangoGirls/tutorial/pull/450

-- 
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/e2c5b5ed-e8f5-4eb2-b608-2e50b46fc89a%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Decoration of class based views

[Fabrizio Messina]

Hello I would like to ask why the class based views documentation seems so 
much ugly. Some developers probably are scared by these just because the 
decoration is ugly, the documentation offers three ways:

Decorate the *Klass().dispach()* method of the class, wrapping the 
decorators in another decorator:

@method_decorator(decorator_1)
def dispatch(request, *args, **kwargs):
  pass

Wrapping the *Klass.as_view()* resulting in an arguably ugly

*:*


*decorator_1( decorator_2( .. ( .. decorator_n( Klass.as_view() ) .. ) .. 
))*In my opinion this is an issue because decoration is actually cool and the 
alternative is to use the much more dreadful Multiple Inheritance, that is 
the documented third way.


I personally use reduce syntax:

view = reduce(lambda x, f: f(x), (decorator_1, decorator_2, .., decorator_n
), Klass.as_view())

Probably there are better ways but still to me it seems to me that 
something like this is more readable, easier to modify and less scary. To 
me it has the pro that one could easily wrap common used decorators in a 
single iterable.

-- 
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/67fd00b2-d3d4-494e-be66-8743bbad49d2%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.