classification
Title: Use Argument Clinic in sqlite3
Type: enhancement Stage: patch review
Components: Library (Lib) Versions: Python 3.10
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: berker.peksag, corona10, erlendaasland, serhiy.storchaka, vstinner
Priority: normal Keywords: patch

Created on 2020-06-12 10:05 by erlendaasland, last changed 2021-02-10 23:04 by berker.peksag.

Pull Requests
URL Status Linked Edit
PR 20826 closed erlendaasland, 2020-06-12 10:10
PR 22484 merged erlendaasland, 2020-10-01 19:37
PR 22478 erlendaasland, 2020-10-13 09:11
PR 23057 closed erlendaasland, 2020-10-31 09:45
PR 23341 merged erlendaasland, 2020-11-17 14:16
PR 23837 merged corona10, 2020-12-18 14:47
PR 23838 merged erlendaasland, 2020-12-18 15:48
PR 23964 merged erlendaasland, 2020-12-27 10:05
PR 24007 merged erlendaasland, 2020-12-30 10:49
PR 24135 closed erlendaasland, 2021-01-06 00:27
PR 24214 closed erlendaasland, 2021-01-13 20:38
PR 24421 open erlendaasland, 2021-02-02 21:03
PR 24503 merged erlendaasland, 2021-02-10 20:54
Messages (21)
msg371347 - (view) Author: Erlend Egeberg Aasland (erlendaasland) * Date: 2020-06-12 10:05
Use Argument Clinic in sqlite3.
msg378558 - (view) Author: Dong-hee Na (corona10) * (Python committer) Date: 2020-10-13 14:00
What's the purpose of using AC, did the change improve performance?
It can make hard to track the code history.
msg378560 - (view) Author: Erlend Egeberg Aasland (erlendaasland) * Date: 2020-10-13 14:38
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)
msg378561 - (view) Author: Erlend Egeberg Aasland (erlendaasland) * Date: 2020-10-13 14:47
Also, see the comment from Victor here: https://github.com/python/cpython/pull/22478#issuecomment-702201260
msg378562 - (view) Author: Dong-hee Na (corona10) * (Python committer) Date: 2020-10-13 15:02
> Also, see the comment from Victor here: https://github.com/python/cpython/pull/22478#issuecomment-702201260

Okay got it
msg378571 - (view) Author: Erlend Egeberg Aasland (erlendaasland) * Date: 2020-10-13 18:16
By the way, what's the preferred way to benchmark performance?
msg380042 - (view) Author: Dong-hee Na (corona10) * (Python committer) Date: 2020-10-31 06:07
New changeset 7d210271579ae31f43b32f73c2aff5bc4fe0d27f by Erlend Egeberg Aasland in branch 'master':
bpo-40956: Convert _sqlite3 module level functions to Argument Clinic (GH-22484)
https://github.com/python/cpython/commit/7d210271579ae31f43b32f73c2aff5bc4fe0d27f
msg380043 - (view) Author: Dong-hee Na (corona10) * (Python committer) Date: 2020-10-31 06:09
@erlendaasland

Thank you erlendaasland for working on GH-22484.
This time was a good time to review my AC knowledge ;)
msg380047 - (view) Author: Erlend Egeberg Aasland (erlendaasland) * Date: 2020-10-31 09:49
> Thank you erlendaasland for working on GH-22484.

… and thank you for taking the time to review this!
msg383301 - (view) Author: Dong-hee Na (corona10) * (Python committer) Date: 2020-12-18 14:25
New changeset 1ba82bbc50a52f40ad05f3c4aaf2e159e0ce126d by Erlend Egeberg Aasland in branch 'master':
bpo-40956: Convert _sqlite3.Connection to Argument Clinic (GH-23341)
https://github.com/python/cpython/commit/1ba82bbc50a52f40ad05f3c4aaf2e159e0ce126d
msg383303 - (view) Author: Dong-hee Na (corona10) * (Python committer) Date: 2020-12-18 15:41
New changeset 2179349d8cf45b1202775547df384b1fde31630a by Dong-hee Na in branch 'master':
bpo-40956: Fix sqlite3 AC code (GH-23837)
https://github.com/python/cpython/commit/2179349d8cf45b1202775547df384b1fde31630a
msg383841 - (view) Author: Dong-hee Na (corona10) * (Python committer) Date: 2020-12-27 08:32
New changeset 3ccef1ca474592e191a00e131dfbaf777db271e9 by Erlend Egeberg Aasland in branch 'master':
bpo-40956: Convert _sqlite3.Connection to Argument Clinic, part 2 (GH-23838)
https://github.com/python/cpython/commit/3ccef1ca474592e191a00e131dfbaf777db271e9
msg384000 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-12-29 13:22
New changeset 84d79cfda947f6bc28a5aa11db8055aa40a6b03a by Erlend Egeberg Aasland in branch 'master':
bpo-40956: Convert _sqlite3.Row to Argument Clinic (GH-23964)
https://github.com/python/cpython/commit/84d79cfda947f6bc28a5aa11db8055aa40a6b03a
msg384444 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2021-01-05 23:57
New changeset c7f8d3caf0b10c19cd46b1b334a98628eac15672 by Erlend Egeberg Aasland in branch 'master':
bpo-40956: Convert _sqlite3.Cursor to Argument Clinic (GH-24007)
https://github.com/python/cpython/commit/c7f8d3caf0b10c19cd46b1b334a98628eac15672
msg385012 - (view) Author: Erlend Egeberg Aasland (erlendaasland) * Date: 2021-01-13 10:32
GH-24007 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
msg385015 - (view) Author: Erlend Egeberg Aasland (erlendaasland) * Date: 2021-01-13 11:00
Looks like this is the culprit: https://github.com/python/cpython/blob/2396614b8958ad202378fd71a598eb4106ac5896/Modules/_sqlite/cursor.c#L825
msg385019 - (view) Author: Erlend Egeberg Aasland (erlendaasland) * Date: 2021-01-13 11:28
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?
msg385036 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2021-01-13 15:12
Use NULL.
msg385053 - (view) Author: Erlend Egeberg Aasland (erlendaasland) * Date: 2021-01-13 20:34
Thanks, but I prefer the following:

size as maxrows: int(c_default='self->arraysize', py_default='<unrepresentable>') = 1
msg385058 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2021-01-13 23:17
New changeset a330365ca5ae836075f306334ab648bf23471481 by Erlend Egeberg Aasland in branch 'master':
bpo-40956: Fix sqlite3.Cursor.fetchmany() default value (GH-24214)
https://github.com/python/cpython/commit/a330365ca5ae836075f306334ab648bf23471481
msg386814 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2021-02-10 23:04
New changeset ea46579067fd2d4e164d6605719ffec690c4d621 by Erlend Egeberg Aasland in branch 'master':
bpo-40956: Fix segfault when Connection.backup is called without target (GH-24503)
https://github.com/python/cpython/commit/ea46579067fd2d4e164d6605719ffec690c4d621
History
Date User Action Args
2021-02-10 23:04:26berker.peksagsetmessages: + msg386814
2021-02-10 20:54:17erlendaaslandsetpull_requests: + pull_request23293
2021-02-02 21:03:36erlendaaslandsetpull_requests: + pull_request23236
2021-01-13 23:17:47berker.peksagsetmessages: + msg385058
2021-01-13 20:38:13erlendaaslandsetpull_requests: + pull_request23041
2021-01-13 20:34:18erlendaaslandsetmessages: + msg385053
2021-01-13 15:12:33serhiy.storchakasetmessages: + msg385036
2021-01-13 11:28:04erlendaaslandsetmessages: + msg385019
2021-01-13 11:00:51erlendaaslandsetmessages: + msg385015
2021-01-13 10:32:24erlendaaslandsetmessages: + msg385012
2021-01-06 00:27:24erlendaaslandsetpull_requests: + pull_request22965
2021-01-05 23:57:52berker.peksagsetmessages: + msg384444
2020-12-30 10:49:04erlendaaslandsetpull_requests: + pull_request22849
2020-12-29 13:22:59serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg384000
2020-12-27 10:05:33erlendaaslandsetpull_requests: + pull_request22809
2020-12-27 08:32:39corona10setmessages: + msg383841
2020-12-18 15:48:48erlendaaslandsetpull_requests: + pull_request22697
2020-12-18 15:41:40corona10setmessages: + msg383303
2020-12-18 14:47:44corona10setpull_requests: + pull_request22696
2020-12-18 14:25:45corona10setmessages: + msg383301
2020-11-17 14:16:14erlendaaslandsetpull_requests: + pull_request22232
2020-10-31 09:49:19erlendaaslandsetmessages: + msg380047
2020-10-31 09:45:37erlendaaslandsetpull_requests: + pull_request21976
2020-10-31 06:09:37corona10setmessages: + msg380043
2020-10-31 06:07:51corona10setmessages: + msg380042
2020-10-13 18:16:14erlendaaslandsetmessages: + msg378571
2020-10-13 15:02:08corona10setmessages: + msg378562
2020-10-13 14:47:46erlendaaslandsetmessages: + msg378561
2020-10-13 14:38:04erlendaaslandsetmessages: + msg378560
2020-10-13 14:00:56corona10setnosy: + corona10
messages: + msg378558
2020-10-13 09:13:28erlendaaslandsetnosy: + vstinner, berker.peksag
2020-10-13 09:11:58erlendaaslandsetpull_requests: + pull_request21654
2020-10-01 19:37:51erlendaaslandsetpull_requests: + pull_request21501
2020-06-12 10:10:15erlendaaslandsetkeywords: + patch
stage: patch review
pull_requests: + pull_request20021
2020-06-12 10:05:35erlendaaslandcreate