gh-101693: In sqlite3, deprecate using named placeholders with parameters supplied as a sequence #101698
Conversation
erlend-aasland
commented
Feb 8, 2023
•
erlend-aasland
commented
- Issue: sqlite3: issue a warning if a sequence of params are used with named placeholders in queries #101693
erlend-aasland
changed the title
This comment was marked as outdated.
This comment was marked as outdated.
erlend-aasland
force-pushed
the
no-numeric-placeholder-for-you
branch
from
25365e9
to
b693895
Compare
|
Alex, would you mind taking a look at the docs/NEWS/What's New changes? I feel the sentences grew to be a little bit too complex. |
|
Also, the text in the deprecation warning is perhaps a little bit too long 😶 |
executemany only works with data-modifying queries
|
Thanks for your review, Serhiy; highly appreciated. @AlexWaygood, do you still want to take a look? |
Misc/NEWS.d/next/Library/2023-02-08-18-20-58.gh-issue-101693.4_LPXj.rst
Outdated
Show resolved
Hide resolved
Doc/library/sqlite3.rst
Outdated
| @@ -1442,6 +1442,12 @@ Cursor objects | |||
| and there is no open transaction, | |||
| a transaction is implicitly opened before executing *sql*. | |||
|
|
|||
| .. versionchanged:: 3.12 | |||
There was a problem hiding this comment.
| .. versionchanged:: 3.12 | |
| .. deprecated-removed:: 3.12 3.14 |
There was a problem hiding this comment.
Are you sure about that? The API itself is not deprecated, we're just changing it. I'm not sure what's the recommended practice here.
There was a problem hiding this comment.
Yeah, I know what you mean. I feel like ideally this notice would be phrased along the lines of "X behaviour/practice is now deprecated" rather than "a DeprecationWarning is now emitted". That would be more to-the-point, and it would also work more naturally with this directive (X behaviour/practice is deprecated in 3.12, and will be removed entirely in 3.14).
But I was struggling to come to with a concrete suggestion for how to reword these notices :/
There was a problem hiding this comment.
(The whatsnew and NEWS entries look great btw, it's just the notices in the API docs that feel slightly clunky to me)
There was a problem hiding this comment.
I feel like ideally this notice would be phrased along the lines of "X behaviour/practice is now deprecated" rather than "a DeprecationWarning is now emitted". That would be more to-the-point, and it would also work more naturally with this directive (X behaviour/practice is deprecated in 3.12, and will be removed entirely in 3.14).
Yes, but documented as deprecated and emitting a DeprecationWarning are similar, but not equal, things 🙂 With the former, we don't need to emit a warning in the code.
There was a problem hiding this comment.
To your original question, btw: I think it's pretty standard to use .. deprecated or .. deprecated-removed, even if it's just a particular usage of an API, rather than the API itself. See e.g. https://docs.python.org/3.12/library/asyncio-policy.html#asyncio.DefaultEventLoopPolicy, where the directive is used even though the class itself hasn't been deprecated at all; or #19867, which deprecated just a specific parameter; or lots of other examples in our docs 🙂
There was a problem hiding this comment.
Aight! I'll try to reword it.
There was a problem hiding this comment.
I've updated it to use deprecated-removed, and I put in an extra line regarding what happens in 3.14. I'm too tired to reword the text 🙂
