Yes, the idea is that ModelForm.save() method could become (after 
deprecation finishes):

def save(self):
    """
    Save this form's self.instance object and M2M data. Return the model
    instance.
    """
    if self.errors:
        raise ValueError(
            "The %s could not be %s because the data didn't validate." % (
                self.instance._meta.object_name,
                'created' if self.instance._state.adding else 'changed',
            )
        )
    self.instance.save()
    self._save_m2m()
    return self.instance

Instead of calling super().save(commit=False) to retrieve form.instance, 
you can just manipulate form.instance directly and then call 
super().save(). I don't see a need for the _instance getters/setters you 
proposed.

There's also a deprecation checklist to help when writing a patch:
https://docs.djangoproject.com/en/1.8/internals/contributing/writing-code/submitting-patches/#deprecating-a-feature

On Monday, August 10, 2015 at 8:30:51 AM UTC-4, Javier Candeira wrote:
>
> On Monday, 10 August 2015 21:36:27 UTC+10, Tim Graham wrote:
>>
>> "I like `.apply()`, since it can either suggest that it's applying the 
>> form's data to the instance, or applies the validation rules to the form 
>> data."
>>
>> I don't think it does either of those things though? I don't find the 
>> name intuitive for what it does. I think the purpose of 
>> "save(commit=False)" is for the case when you need to modify a model 
>> instance *outside* of the form. If you can modify the instance inside the 
>> form, just do so in save().
>>
>
> So the only thing it does is throw an exception if there is a validation 
> error, and prepare m2m() so the m2m data will be saved later? 
>  
>
>> What happened to the idea of removing the need for the commit keyword arg 
>> that you presented earlier? As I said, I thought that was pretty good, 
>> except some investigation of whether or not it'll work well for formsets is 
>> needed.
>>
>
> Good catch. I got distracted rebasing from your patch, then I realised I 
> don't know how deprecation works inside the Django project, and somewhere 
> along the way I guess I suffered a loss of ambition.
>
> I'll give the patch another spin, try to understand what the commit=False 
> codepath does that can be done when instantiating the form so save() 
> doesn't need the commit keyword. 
>
> What's your opinion on turning `ModelForm.instance` into a property that 
> would get/set an _instance attribute, and perform the same side effects as 
> save(commit=False) perform now when returning the model instance? To me 
> this looks like it could work with `FormSet.instances` too, and it would 
> avoid having to find a name for the method, since `apply` and 
> `get_updated_instance` did not work. `.instance` just gets and sets 
> instance/instances, and does whatever preparation is needed in the 
> background.
>
> Also, is there anywhere I have missed in the documentation that explains 
> how deprecation works in the project? If we go for such a break with the 
> existing API, my patch would need to keep the current API working for a 
> while, and the documentation would have to document both APIs in parallel 
> for that time, then deprecate the `commit=False` path.
>
> Thanks,
>
> J
>
>
> On Sunday, August 9, 2015 at 7:18:57 PM UTC-4, Javier Candeira wrote:
>>>
>>> On Saturday, 8 August 2015 02:38:09 UTC+10, Patryk Zawadzki wrote:
>>>>
>>>> 2015-08-07 16:51 GMT+02:00 Martin Owens <doct...@gmail.com>: 
>>>> > Could we use something simple like 'update()' and 'commit()' to which 
>>>> save 
>>>> > would call both? 
>>>>
>>>> Or `.apply()` and `.commit()` not to suggest that the form gets 
>>>> updated in any way. 
>>>>
>>>
>>> I like `.apply()`, since it can either suggest that it's applying the 
>>> form's data to the instance, or applies the validation rules to the form 
>>> data. 
>>>
>>> J
>>>
>>

-- 
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/1c1e50c3-96f4-4ef1-b5b1-a95e5d89be24%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to