Skip to content

Docs: use parameter list for sqlite3.Cursor.execute*#101782

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
erlend-aasland merged 5 commits into from
Feb 10, 2023
Merged

Docs: use parameter list for sqlite3.Cursor.execute* #101782

erlend-aasland merged 5 commits into from
Feb 10, 2023

Conversation

@erlend-aasland
Copy link
Contributor

@erlend-aaslanderlend-aasland commented Feb 10, 2023

No description provided.

@bedevere-botbedevere-bot added awaiting core review docs Documentation in the Doc dir labels Feb 10, 2023
@erlend-aasland
Copy link
ContributorAuthor

erlend-aasland commented Feb 10, 2023

I'm not sure if this is an improvement; I need new eyes to tell me :) Also, I'm in doubt of how to use the :raises Sphinx param here; execute*() can raise a heck of a lot of exceptions; should we write all of them down here, or just a subset? Perhaps just point to the reference for sqlite3 exceptions?

AlexWaygood
AlexWaygood reviewed Feb 10, 2023
View reviewed changes
Copy link
Member

@AlexWaygoodAlexWaygood left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a nice improvement! I find it much easier to understand when parameters should be a sequence and when it should be a dict with this change

AlexWaygood
AlexWaygood reviewed Feb 10, 2023
View reviewed changes
erlend-aaslandand others added 2 commits February 10, 2023 17:58
@erlend-aasland@AlexWaygood
Update Doc/library/sqlite3.rst
bedcfd3
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
@erlend-aasland
... or iterator
ed08eab
there's a slight difference
AlexWaygood
AlexWaygood reviewed Feb 10, 2023
View reviewed changes
Copy link
Member

@AlexWaygoodAlexWaygood left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The set of "all Python iterables" is a superset of the set of "all Python sequences" and a superset of the set of "all Python sequences". All sequences are by definition also iterables; all iterators are by definition also iterables.

(sorry, reviewing on my phone — think I screwed up my last suggestion!)

@erlend-aasland
Copy link
ContributorAuthor

erlend-aasland commented Feb 10, 2023

Correct! I had to go re-read the glossary. Thanks for the heads-up.

@AlexWaygood
Copy link
Member

AlexWaygood commented Feb 10, 2023

Correct! I had to go re-read the glossary. Thanks for the heads-up.

Pro tip: reading the source code for _collections_abc.py is often more informative than reading the glossary entry, I find!

AlexWaygood
AlexWaygood approved these changes Feb 10, 2023
View reviewed changes
Copy link
Member

@AlexWaygoodAlexWaygood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@erlend-aasland
Copy link
ContributorAuthor

erlend-aasland commented Feb 10, 2023

Pro tip: reading the source code for _collections_abc.py is often more informative than reading the glossary entry, I find!

Ouch, we can't have it that way; in that case we need to improve the glossary. I haven't read a lot of the .py files in Lib; I'm more into the Modules dir :) But I'll take a look; thanks for the tip!

@erlend-aasland
Copy link
ContributorAuthor

erlend-aasland commented Feb 10, 2023

Thanks for the helpful review; highly appreciated!

@erlend-aaslanderlend-aasland merged commit 2037ebf into python:mainFeb 10, 2023
@miss-islington
Copy link
Contributor

miss-islington commented Feb 10, 2023

Thanks @erlend-aasland for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

miss-islington commented Feb 10, 2023

Sorry, @erlend-aasland, I could not cleanly backport this to 3.10 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 2037ebf81bd4bbe5421421b822bd57cfd665a1e9 3.10

@bedevere-bot
Copy link

bedevere-bot commented Feb 10, 2023

GH-101803 is a backport of this pull request to the 3.11 branch.

@bedevere-botbedevere-bot removed the needs backport to 3.11 only security fixes label Feb 10, 2023
@erlend-aaslanderlend-aasland deleted the sqlite-docs/execute-params-best-practice branch February 10, 2023 17:54
@erlend-aasland
Copy link
ContributorAuthor

erlend-aasland commented Feb 10, 2023

(I'll fix the 3.10 backport later today)

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 10, 2023
@erlend-aasland@AlexWaygood@miss-islington
Docs: use parameter list for sqlite3.Cursor.execute* (pythonGH-101782)
16e0b88
(cherry picked from commit 2037ebf) Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com> Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
@AlexWaygood
Copy link
Member

AlexWaygood commented Feb 10, 2023

Ouch, we can't have it that way; in that case we need to improve the glossary.

I know what you mean, but I actually wonder if it's possible in this case to convey the same information as tersely and as precisely in the glossary. I find the inheritance structures in _collections_abc.py very descriptive and quite elegant. It's possible you could replicate that with some kind of diagram in the docs somewhere, but I'm not sure.

There's also the fact that it's nice to have all the collections-related concepts described together in a separate file. The glossary interspersed these terms with completely unrelated concepts (as a glossary should! This is its nature — but for understanding these concepts specifically, I find it unhelpful)

miss-islington added a commit that referenced this pull request Feb 10, 2023
@miss-islington@erlend-aasland@AlexWaygood
Docs: use parameter list for sqlite3.Cursor.execute* (GH-101782)
18313ec
(cherry picked from commit 2037ebf) Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com> Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
carljm added a commit to carljm/cpython that referenced this pull request Feb 10, 2023
@carljm
Merge branch 'main' into inlinecomp2
36b2917
* main: Docs: Fix getstatus() -> getcode() typos (python#101296) Docs: use parameter list for sqlite3.Cursor.execute* (python#101782) pythongh-101763: Update bundled copy of libffi to 3.4.4 on Windows (pythonGH-101784) pythongh-101517: make bdb avoid looking up in linecache with lineno=None (python#101787) pythongh-101759: Update Windows installer to SQLite 3.40.1 (python#101762) pythongh-101277: Finalise isolating itertools (pythonGH-101305) pythongh-101759: Update macOS installer to SQLite 3.40.1 (python#101761)
erlend-aasland added a commit to erlend-aasland/cpython that referenced this pull request Feb 10, 2023
@erlend-aasland
[3.10] Docs: use parameter list for sqlite3.Cursor.execute* (pythonGH…
dca2822
…-101782) Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>. (cherry picked from commit 2037ebf) Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
@bedevere-bot
Copy link

bedevere-bot commented Feb 10, 2023

GH-101808 is a backport of this pull request to the 3.10 branch.

@bedevere-botbedevere-bot removed the needs backport to 3.10 only security fixes label Feb 10, 2023
erlend-aasland added a commit that referenced this pull request Feb 10, 2023
@erlend-aasland
[3.10] Docs: use parameter list for sqlite3.Cursor.execute* (GH-101782)…
a565cd5
… (#101808) Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com> (cherry picked from commit 2037ebf)
@tom93tom93 mentioned this pull request Nov 27, 2023
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AlexWaygoodAlexWaygoodAlexWaygood approved these changes

@berkerpeksagberkerpeksagAwaiting requested review from berkerpeksagberkerpeksag is a code owner

Assignees

@erlend-aaslanderlend-aasland

Labels

docsDocumentation in the Doc dirskip issueskip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@erlend-aasland@AlexWaygood@miss-islington@bedevere-bot