[Django] #35060: Make Model.save() arguments keyword-only

[Django]

#35060: Make Model.save() arguments keyword-only
-------------------------------------+-------------------------------------
Reporter: Jacob | Owner: Jacob Walls
Walls |
Type: | Status: assigned
Cleanup/optimization |
Component: | Version: dev
Uncategorized |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Following this forum [https://forum.djangoproject.com/t/hook-for-
extending-update-fields-in-save-overrides/26040/5 discussion], I suggest
making `Model.save()` accept keyword arguments only (following a
deprecation period, of course).

For users who need to patch `update_fields` in an overridden `save()`, it
would be much simpler to only have to drill into `**kwargs` and not also
drill into (and check the length of) `*args` first.

Implementing this ticket would alleviate a small part of a current
inconsistency in the advice for overridding save(). In one portion, the
implementer is advised to pass through both `*args` and `**kwargs`:

{{{
It's also important that you pass through the arguments that can be
passed to the model method -- that's what the ``*args, **kwargs`` bit
does. Django will, from time to time, extend the capabilities of
built-in model methods, adding new arguments. If you use ``*args,
**kwargs`` in your method definitions, you are guaranteed that your
code will automatically support those arguments when they are added.
}}}

Then, an example tailored to passing through `update_fields`, passes
through *no* variadic arguments:
{{{
def save(
self, force_insert=False, force_update=False, using=None,
update_fields=None
):
}}}

We could make the situation better by taking away swallowed `*args` as a
vector for bugs.

--
Ticket URL: <https://code.djangoproject.com/ticket/35060>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#35060: Make Model.save() arguments keyword-only
-------------------------------------+-------------------------------------

Reporter: Jacob Walls | Owner: Jacob
Type: | Walls
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jacob Walls):

* component: Uncategorized => Database layer (models, ORM)

--
Ticket URL: <https://code.djangoproject.com/ticket/35060#comment:1>

[Django]

#35060: Make Model.save() arguments keyword-only
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Jacob
Type: | Walls
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:

Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak):

* stage: Unreviewed => Accepted

Comment:

Sounds reasonable.

--
Ticket URL: <https://code.djangoproject.com/ticket/35060#comment:2>

[Django]

#35060: Make Model.save() arguments keyword-only
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Jacob
Type: | Walls
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Aryan Gholizadeh Mojaveri):

May we create a PR for this now?

--
Ticket URL: <https://code.djangoproject.com/ticket/35060#comment:3>

[Django]

#35060: Make Model.save() arguments keyword-only
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Jacob
Type: | Walls
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Jacob Walls):

I haven't started, so if you'd like to assign the ticket to yourself,
that's fine with me. (Be sure to follow the "Deprecating a feature"
instructions and update the two documentation examples I mentioned above.)

--
Ticket URL: <https://code.djangoproject.com/ticket/35060#comment:4>

[Django]

#35060: Make Model.save() arguments keyword-only
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Jacob
Type: | Walls
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Adam Johnson):

* cc: Adam Johnson (added)

Comment:

Nice one. If this ticket goes well, it might serve as a template for
making some other functions keyword-only, such as `models.Field.__init__`
(except the first two args, `verbose_name` and `name`).

--
Ticket URL: <https://code.djangoproject.com/ticket/35060#comment:5>

[Django]

#35060: Make Model.save() arguments keyword-only
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Jacob
Type: | Walls
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak):

Replying to [comment:5 Adam Johnson]:

> Nice one. If this ticket goes well, it might serve as a template for
making some other functions keyword-only, such as `models.Field.__init__`
(except the first two args, `verbose_name` and `name`).

We already have "templates" for such deprecations e.g.
ad18a0102cc2968914232814c6554763f15abbe3 or
b8738aea14446b267a47087b52b38a98b440a6aa.

--
Ticket URL: <https://code.djangoproject.com/ticket/35060#comment:6>

[Django]

#35060: Make Model.save() arguments keyword-only
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Jacob
Type: | Walls
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Salvo Polizzi):

* has_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/35060#comment:7>

[Django]

#35060: Make Model.save() arguments keyword-only
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Jacob
Type: | Walls
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Salvo Polizzi):

* has_patch: 1 => 0

Comment:

I'm sorry but some tests do not work, so I temporarily closed PR. I will
fix them

--
Ticket URL: <https://code.djangoproject.com/ticket/35060#comment:8>

[Django]

#35060: Make Model.save() arguments keyword-only
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Jacob
Type: | Walls
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Salvo Polizzi):

* has_patch: 0 => 1

Comment:

Replying to [comment:8 Salvo Polizzi]:

> I'm sorry but some tests do not work, so I temporarily closed PR. I will
fix them

--
Ticket URL: <https://code.djangoproject.com/ticket/35060#comment:9>

[Django]

#35060: Make Model.save() arguments keyword-only
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Jacob
Type: | Walls
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by David Wobrock):

* cc: David Wobrock (added)
* needs_better_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/35060#comment:10>

[Django]

#35060: Make Model.save() arguments keyword-only
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Jacob
Type: | Walls
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Salvo Polizzi):

* needs_better_patch: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/35060#comment:11>

[Django]

#35060: Make Model.save() arguments keyword-only
-------------------------------------+-------------------------------------

Reporter: Jacob Walls | Owner: Salvo
Type: | Polizzi

Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jacob Walls):

* owner: Jacob Walls => Salvo Polizzi

* needs_better_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/35060#comment:12>

[Django]

#35060: Make Model.save() arguments keyword-only
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Salvo
Type: | Polizzi
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Salvo Polizzi):

* needs_better_patch: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/35060#comment:13>

[Django]

#35060: Make Model.save() arguments keyword-only
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Salvo
Type: | Polizzi
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:

Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/35060#comment:14>

[Django]

#35060: Make Model.save() arguments keyword-only
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Salvo
Type: | Polizzi

Cleanup/optimization | Status: closed

Component: Database layer | Version: dev
(models, ORM) |

Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak <felisiak.mariusz@…>):

* status: assigned => closed
* resolution: => fixed

Comment:

In [changeset:"3915d4c70d0d7673abe675525b58117a5099afd3" 3915d4c]:
{{{
#!CommitTicketReference repository=""
revision="3915d4c70d0d7673abe675525b58117a5099afd3"
Fixed #35060 -- Deprecated passing positional arguments to
Model.save()/asave().
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/35060#comment:15>

[Django]

#35060: Make Model.save() arguments keyword-only
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Salvo
Type: | Polizzi
Cleanup/optimization | Status: closed
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by GitHub <noreply@…>):

In [changeset:"28522c3c8d5eb581347aececc3ac61c134528114" 28522c3]:
{{{#!CommitTicketReference repository=""
revision="28522c3c8d5eb581347aececc3ac61c134528114"
Fixed #35554, Refs #35060 -- Corrected deprecated *args parsing in
Model.save()/asave().

The transitional logic added to deprecate the usage of *args for
Model.save()/asave() introduced two issues that this branch fixes:
* Passing extra positional arguments no longer raised TypeError.
* Passing a positional but empty update_fields would save all fields.

Co-authored-by: Natalia <124304+...@users.noreply.github.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35060#comment:16>

[Django]

#35060: Make Model.save() arguments keyword-only
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Salvo
Type: | Polizzi
Cleanup/optimization | Status: closed
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Natalia <124304+nessita@…>):

In [changeset:"387172918f65762578d93a9b3f94d20209f7a769" 38717291]:
{{{#!CommitTicketReference repository=""
revision="387172918f65762578d93a9b3f94d20209f7a769"
[5.1.x] Fixed #35554, Refs #35060 -- Corrected deprecated *args parsing in

Model.save()/asave().

The transitional logic added to deprecate the usage of *args for
Model.save()/asave() introduced two issues that this branch fixes:
* Passing extra positional arguments no longer raised TypeError.
* Passing a positional but empty update_fields would save all fields.

Co-authored-by: Natalia <124304+...@users.noreply.github.com>

Backport of 28522c3c8d5eb581347aececc3ac61c134528114 from main.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35060#comment:17>