Use Argument Clinic in sqlite3

[erlend-aasland]

BPO	40956
Nosy	@vstinner, @berkerpeksag, @serhiy-storchaka, @corona10, @pablogsal, @erlend-aasland
PRs	bpo-40956: Use Argument Clinic in sqlite3 module #20826 bpo-40956: Convert _sqlite3 module level functions to Argument Clinic #22484 bpo-41861: Convert _sqlite3 CursorType and ConnectionType to heap types #22478 bpo-40956: Convert _sqlite3.Connection to Argument Clinic #23057 bpo-40956: Convert _sqlite3.Connection to Argument Clinic, part 1 of 2 #23341 bpo-40956: Fix sqlite3 AC code #23837 bpo-40956: Convert _sqlite3.Connection to Argument Clinic, part 2 #23838 bpo-40956: Convert _sqlite3.Row to Argument Clinic #23964 bpo-40956: Convert _sqlite3.Cursor to Argument Clinic #24007 bpo-40956: Convert _sqlite3.Cache to Argument Clinic #24135 bpo-40956: fix sqlite3.Cursor.fetchmany() default value #24214 bpo-40956: Convert sqlite3.connect and sqlite3.Connection.__init__ to AC #24421 bpo-40956: [regression] first argument in sqlite3.Connection.backup is mandatory #24503

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2021-06-20.20:29:03.608>
created_at = <Date 2020-06-12.10:05:35.702>
labels = ['type-feature', 'library', '3.10']
title = 'Use Argument Clinic in sqlite3'
updated_at = <Date 2021-06-20.20:29:03.605>
user = 'https://github.com/erlend-aasland'

bugs.python.org fields:

activity = <Date 2021-06-20.20:29:03.605>
actor = 'erlendaasland'
assignee = 'none'
closed = True
closed_date = <Date 2021-06-20.20:29:03.608>
closer = 'erlendaasland'
components = ['Library (Lib)']
creation = <Date 2020-06-12.10:05:35.702>
creator = 'erlendaasland'
dependencies = []
files = []
hgrepos = []
issue_num = 40956
keywords = ['patch']
message_count = 24.0
messages = ['371347', '378558', '378560', '378561', '378562', '378571', '380042', '380043', '380047', '383301', '383303', '383841', '384000', '384444', '385012', '385015', '385019', '385036', '385053', '385058', '386814', '396188', '396191', '396197']
nosy_count = 6.0
nosy_names = ['vstinner', 'berker.peksag', 'serhiy.storchaka', 'corona10', 'pablogsal', 'erlendaasland']
pr_nums = ['20826', '22484', '22478', '23057', '23341', '23837', '23838', '23964', '24007', '24135', '24214', '24421', '24503']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue40956'
versions = ['Python 3.10']

[erlend-aasland]

Use Argument Clinic in sqlite3.

[corona10]

What's the purpose of using AC, did the change improve performance?
It can make hard to track the code history.

[erlend-aasland]

The primary reason is that it will be provide easy access to module state.

The first step in making sqlite3 support multiphase init was to create heap types. The second step is argument clinic. The third will be to use AC for module state. The last step will then be final multiphase support.

Also, IMHO, AC greatly improves the body of methods (readability => maintainability, hardened parsing of arguments)

[erlend-aasland]

Also, see the comment from Victor here: #22478 (comment)

[corona10]

Also, see the comment from Victor here: #22478 (comment)

Okay got it

[erlend-aasland]

By the way, what's the preferred way to benchmark performance?

[corona10]

New changeset 7d21027 by Erlend Egeberg Aasland in branch 'master':
bpo-40956: Convert _sqlite3 module level functions to Argument Clinic (GH-22484)
7d21027

[corona10]

@erlendaasland

Thank you erlendaasland for working on #66674.
This time was a good time to review my AC knowledge ;)

[erlend-aasland]

Thank you erlendaasland for working on #66674.

… and thank you for taking the time to review this!

[corona10]

New changeset 1ba82bb by Erlend Egeberg Aasland in branch 'master':
bpo-40956: Convert _sqlite3.Connection to Argument Clinic (GH-23341)
1ba82bb

[corona10]

New changeset 2179349 by Dong-hee Na in branch 'master':
bpo-40956: Fix sqlite3 AC code (GH-23837)
2179349

[corona10]

New changeset 3ccef1c by Erlend Egeberg Aasland in branch 'master':
bpo-40956: Convert _sqlite3.Connection to Argument Clinic, part 2 (GH-23838)
3ccef1c

[serhiy-storchaka]

New changeset 84d79cf by Erlend Egeberg Aasland in branch 'master':
bpo-40956: Convert _sqlite3.Row to Argument Clinic (GH-23964)
84d79cf

[berkerpeksag]

New changeset c7f8d3c by Erlend Egeberg Aasland in branch 'master':
bpo-40956: Convert _sqlite3.Cursor to Argument Clinic (GH-24007)
c7f8d3c

[erlend-aasland]

#68195 introduced a regression:

>>> import sqlite3
>>> help(sqlite3)
Traceback (most recent call last):
  File "Lib/inspect.py", line 2049, in wrap_value
    value = eval(s, module_dict)
  File "<string>", line 1, in <module>
NameError: name 'cursor' is not defined

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "Lib/inspect.py", line 2052, in wrap_value
    value = eval(s, sys_module_dict)
  File "<string>", line 1, in <module>
NameError: name 'cursor' is not defined

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "Lib/_sitebuiltins.py", line 103, in __call__
    return pydoc.help(*args, **kwds)
  File "Lib/pydoc.py", line 2000, in __call__
    self.help(request)
  File "Lib/pydoc.py", line 2059, in help
    else: doc(request, 'Help on %s:', output=self._output)
  File "Lib/pydoc.py", line 1779, in doc
    pager(render_doc(thing, title, forceload))
  File "Lib/pydoc.py", line 1772, in render_doc
    return title % desc + '\n\n' + renderer.document(object, name)
  File "Lib/pydoc.py", line 472, in document
    if inspect.ismodule(object): return self.docmodule(*args)
  File "Lib/pydoc.py", line 1267, in docmodule
    contents.append(self.document(value, key, name))
  File "Lib/pydoc.py", line 473, in document
    if inspect.isclass(object): return self.docclass(*args)
  File "Lib/pydoc.py", line 1433, in docclass
    attrs = spill("Methods %s:\n" % tag, attrs,
  File "Lib/pydoc.py", line 1382, in spill
    push(self.document(value,
  File "Lib/pydoc.py", line 474, in document
    if inspect.isroutine(object): return self.docroutine(*args)
  File "Lib/pydoc.py", line 1492, in docroutine
    signature = inspect.signature(object)
  File "Lib/inspect.py", line 3129, in signature
    return Signature.from_callable(obj, follow_wrapped=follow_wrapped,
  File "Lib/inspect.py", line 2877, in from_callable
    return _signature_from_callable(obj, sigcls=cls,
  File "Lib/inspect.py", line 2351, in _signature_from_callable
    return _signature_from_builtin(sigcls, obj,
  File "Lib/inspect.py", line 2164, in _signature_from_builtin
    return _signature_fromstr(cls, func, s, skip_bound_arg)
  File "Lib/inspect.py", line 2102, in _signature_fromstr
    p(name, default)
  File "Lib/inspect.py", line 2084, in p
    default_node = RewriteSymbolics().visit(default_node)
  File "Lib/ast.py", line 410, in visit
    return visitor(node)
  File "Lib/inspect.py", line 2071, in visit_Attribute
    return wrap_value(value)
  File "Lib/inspect.py", line 2054, in wrap_value
    raise RuntimeError()
RuntimeError

[erlend-aasland]

Looks like this is the culprit:

cpython/Modules/_sqlite/cursor.c

Line 825 in 2396614

size as maxrows: int(c_default='self->arraysize') = cursor.arraysize

[erlend-aasland]

I don't see how to easily solve this in AC: we want the arraysize of the cursor instance, but I don't see how I'm going to get this via the AC namespace/context. Using a "bogus" default value works:

size as maxrows: int(c_default='self->arraysize') = 1

Is there a better way?

[serhiy-storchaka]

Use NULL.

[erlend-aasland]

Thanks, but I prefer the following:

size as maxrows: int(c_default='self->arraysize', py_default='<unrepresentable>') = 1

[berkerpeksag]

New changeset a330365 by Erlend Egeberg Aasland in branch 'master':
bpo-40956: Fix sqlite3.Cursor.fetchmany() default value (GH-24214)
a330365

[berkerpeksag]

New changeset ea46579 by Erlend Egeberg Aasland in branch 'master':
bpo-40956: Fix segfault when Connection.backup is called without target (GH-24503)
ea46579

[pablogsal]

New changeset 185ecdc by Erlend Egeberg Aasland in branch 'main':
bpo-40956: Convert sqlite3.connect and sqlite3.Connection.__init__ to AC (GH-24421)
185ecdc

[pablogsal]

Erlend, is anything left in this issue?

[erlend-aasland]

Erlend, is anything left in this issue?

Nothing left; thank you for your guidance and reviews, Dong-hee, Berker, Serhiy, and Pablo.