Skip to content
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

gh-95273: Move sqlite3 executemany examples from reference to tutorial #95351

Merged
merged 6 commits into from
Jul 29, 2022
Merged

gh-95273: Move sqlite3 executemany examples from reference to tutorial #95351

merged 6 commits into from
Jul 29, 2022

Conversation

erlend-aasland
Copy link
Contributor

@erlend-aasland erlend-aasland commented Jul 27, 2022 •
edited by bedevere-bot
Loading

@erlend-aasland erlend-aasland mentioned this pull request Jul 27, 2022
Closed
5 tasks
@erlend-aasland
Copy link
Contributor Author

My 2 cents:

The executemany examples are way more complex than the average user needs. We should focus on the sqlite3 module; not on writing fancy iterators.

The only thing that bothers me, is that we do not explicitly mention that the cursor can be used as an iterator. We could add a mention in the cursor class description.

CAM-Gerlach
CAM-Gerlach reviewed Jul 28, 2022
View reviewed changes
Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

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

A couple minor comments, otherwise LGTM. Thanks @erlend-aasland !

Erlend Egeberg Aasland and others added 2 commits July 28, 2022 08:40
@erlend-aasland
Copy link
Contributor Author

erlend-aasland commented Jul 28, 2022 •
edited
Loading

The only thing that bothers me, is that we do not explicitly mention that the cursor can be used as an iterator. We could add a mention in the cursor class description.

I've already got this covered in the #95273 todo list. UPDATE: PR => #95379

@AlexWaygood
Copy link
Member

I've got quite a lot on at the moment, so I'll leave this one in @CAM-Gerlach's very capable hands, if that's okay!

@AlexWaygood AlexWaygood removed their request for review July 28, 2022 07:27
@erlend-aasland
Copy link
Contributor Author

I wonder if it may make sense to keep a minimum example in the reference, though. The two previous examples ate more than a screenful. How about leaving something like this:

data = [
    ("row1",),
    ("row2",),
]
cur.executemany("insert into t values(?)", data)

@CAM-Gerlach
Copy link
Member

CAM-Gerlach commented Jul 29, 2022 •
edited
Loading

How about leaving something like this:

SGTM; concise examples can certainly be helpful to aiding understanding and this one certainly helps me understand the purpose of executemany much better (at least as a relative novice)

CAM-Gerlach
CAM-Gerlach approved these changes Jul 29, 2022
View reviewed changes
Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @erlend-aasland 👍

@erlend-aasland erlend-aasland merged commit f0bf795 into python:main Jul 29, 2022
@erlend-aasland erlend-aasland deleted the sqlite-reference/extract-executemany-example branch July 29, 2022 07:42
@erlend-aasland erlend-aasland added needs backport to 3.10 only security fixes needs backport to 3.11 only security fixes labels Jul 29, 2022
@miss-islington
Copy link
Contributor

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

@miss-islington
Copy link
Contributor

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 29, 2022
@miss-islington
pythongh-95273: Move sqlite3 executemany examples from reference to t…
946ab9c 
…utorial (pythonGH-95351)

(cherry picked from commit f0bf795)

Co-authored-by: Erlend Egeberg Aasland <[email protected]>
@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Jul 29, 2022
@bedevere-bot
Copy link

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 29, 2022
@miss-islington
pythongh-95273: Move sqlite3 executemany examples from reference to t…
5b1044d 
…utorial (pythonGH-95351)

(cherry picked from commit f0bf795)

Co-authored-by: Erlend Egeberg Aasland <[email protected]>
@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Jul 29, 2022
@bedevere-bot
Copy link

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

miss-islington added a commit that referenced this pull request Jul 29, 2022
@miss-islington
gh-95273: Move sqlite3 executemany examples from reference to tutorial (
f59a5c5 
GH-95351)

(cherry picked from commit f0bf795)

Co-authored-by: Erlend Egeberg Aasland <[email protected]>
miss-islington added a commit that referenced this pull request Jul 29, 2022
@miss-islington
gh-95273: Move sqlite3 executemany examples from reference to tutorial (
f06f365 
GH-95351)

(cherry picked from commit f0bf795)

Co-authored-by: Erlend Egeberg Aasland <[email protected]>
@erlend-aasland erlend-aasland linked an issue Aug 1, 2022 that may be closed by this pull request
Improve the sqlite3 reference #95273
Closed
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CAM-Gerlach CAM-Gerlach CAM-Gerlach approved these changes

@berkerpeksag berkerpeksag Awaiting requested review from berkerpeksag berkerpeksag is a code owner

Assignees
No one assigned
Labels
skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Improve the sqlite3 reference
5 participants
@erlend-aasland @AlexWaygood @CAM-Gerlach @miss-islington @bedevere-bot