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.
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.
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
bpo-40956: Convert sqlite3.connect and sqlite3.Connection.__init__ to AC #24421
bpo-40956: Convert sqlite3.connect and sqlite3.Connection.__init__ to AC #24421
Changes from all commits
8192606
b72a29c
d361aeb
a861358
1ccf5e5
07164cb
a1e88c9
4977d78
1b2720f
c7cd9ee
39ed36b
754fdc6
e59af3b
ad345e0
af0fc7e
98f1a9d
89926af
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Hummmm, i don't fully get how this is preserving the state as before. Seems that the previous code didn't have a default for factory and used NULL as the c default. Could you briefly explain how this default is preserving previous behaviour?
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.
Sure :) This is the current code:
cpython/Modules/_sqlite/module.c
Lines 92 to 95 in 1d10bf0
In practice, the
factory
default isConnectionType
, notNULL
, so I chose to let AC take care of this instead of writing it out explicitly in the function body. This also aligns well with the documentation, which says thatfactory
defaults to thesqlite3.Connection
type.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.
Oh, I see. Is other code using this branch in
module.c:92
? I winder if we can enforce that a factory is always provided after this PRThere 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.
Not in the stdlib (except for some tests in the test suite), but I guess it's used in the wild. Personally, I would not have added this feature in the first place.
That's kind of what we're doing no with the default AC argument, right?