GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Docs: use parameter list for sqlite3.Cursor.execute* by erlend-aasland · Pull Request #101782 · python/cpython

Conversation

@erlend-aasland

@erlend-aasland erlend-aasland commented Feb 10, 2023

Copy link
Copy Markdown
Contributor

No description provided.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@erlend-aasland
Docs: use parameter list for sqlite3.Cursor.execute*
84aa865
@erlend-aasland
erlend-aasland requested a review from berkerpeksag as a code owner February 10, 2023 13:23
@erlend-aasland erlend-aasland added skip issue skip news needs backport to 3.10 needs backport to 3.11 only security fixes labels Feb 10, 2023
@erlend-aasland
erlend-aasland requested a review from AlexWaygood February 10, 2023 13:23
@bedevere-bot bedevere-bot added awaiting core review docs Documentation in the Doc dir labels Feb 10, 2023
@erlend-aasland

erlend-aasland commented Feb 10, 2023

Copy link
Copy Markdown
Contributor Author

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?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
AlexWaygood
AlexWaygood reviewed Feb 10, 2023
View reviewed changes

@AlexWaygood AlexWaygood left a comment •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
erlend-aasland reacted with hooray emoji
All reactions
  • 🎉 1 reaction
Comment thread Doc/library/sqlite3.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/library/sqlite3.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/library/sqlite3.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/library/sqlite3.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@erlend-aasland
Address review
9d1fb1d
AlexWaygood
AlexWaygood reviewed Feb 10, 2023
View reviewed changes
Comment thread Doc/library/sqlite3.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
erlend-aasland and others added 2 commits February 10, 2023 17:58
@erlend-aasland @AlexWaygood
Update Doc/library/sqlite3.rst
bedcfd3 
Co-authored-by: Alex Waygood <[email protected]>
@erlend-aasland
... or iterator
ed08eab 
there's a slight difference
AlexWaygood
AlexWaygood reviewed Feb 10, 2023
View reviewed changes

@AlexWaygood AlexWaygood left a comment •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Doc/library/sqlite3.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/library/sqlite3.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@erlend-aasland

erlend-aasland commented Feb 10, 2023

Copy link
Copy Markdown
Contributor Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@erlend-aasland
Amend
9dcc5da
@AlexWaygood

AlexWaygood commented Feb 10, 2023

Copy link
Copy Markdown
Member

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!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
AlexWaygood
AlexWaygood approved these changes Feb 10, 2023
View reviewed changes

@AlexWaygood AlexWaygood left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Looks great!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
erlend-aasland reacted with hooray emoji
All reactions
  • 🎉 1 reaction
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels Feb 10, 2023
@erlend-aasland

erlend-aasland commented Feb 10, 2023

Copy link
Copy Markdown
Contributor Author

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!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@erlend-aasland

erlend-aasland commented Feb 10, 2023

Copy link
Copy Markdown
Contributor Author

Thanks for the helpful review; highly appreciated!
AlexWaygood reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@erlend-aasland
erlend-aasland merged commit 2037ebf into python:main Feb 10, 2023
@miss-islington

miss-islington commented Feb 10, 2023

Copy link
Copy Markdown
Contributor

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the awaiting merge label Feb 10, 2023
@miss-islington

miss-islington commented Feb 10, 2023

Copy link
Copy Markdown
Contributor

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington miss-islington assigned erlend-aasland Feb 10, 2023
@bedevere-bot

bedevere-bot commented Feb 10, 2023

Copy link
Copy Markdown

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Feb 10, 2023
@erlend-aasland
erlend-aasland deleted the sqlite-docs/execute-params-best-practice branch February 10, 2023 17:54
@erlend-aasland

erlend-aasland commented Feb 10, 2023

Copy link
Copy Markdown
Contributor Author

(I'll fix the 3.10 backport later today)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
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 <[email protected]>
Co-authored-by: Alex Waygood <[email protected]>
@AlexWaygood

AlexWaygood commented Feb 10, 2023

Copy link
Copy Markdown
Member

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)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
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 <[email protected]>
Co-authored-by: Alex Waygood <[email protected]>
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 <[email protected]>.
(cherry picked from commit 2037ebf)

Co-authored-by: Erlend E. Aasland <[email protected]>
@bedevere-bot

bedevere-bot commented Feb 10, 2023

Copy link
Copy Markdown

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the needs backport to 3.10 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 <[email protected]>

(cherry picked from commit 2037ebf)
@tom93 tom93 mentioned this pull request Nov 27, 2023
Docs: fix typo in doc for sqlite3.Cursor.execute #112442
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AlexWaygood AlexWaygood AlexWaygood approved these changes
@berkerpeksag berkerpeksag Awaiting requested review from berkerpeksag

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@erlend-aasland erlend-aasland

Labels

docs Documentation in the Doc dir skip issue skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@erlend-aasland @AlexWaygood @miss-islington @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.