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.

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

sqlite3 signature discrepancies between documentation and implementation #87260

Closed
nchammas mannequin opened this issue Feb 1, 2021 · 12 comments · Fixed by #93840
Closed

sqlite3 signature discrepancies between documentation and implementation #87260

nchammas mannequin opened this issue Feb 1, 2021 · 12 comments · Fixed by #93840
Labels
3.9 3.10 docs expert-sqlite3

Comments

@nchammas
Copy link
Mannequin

@nchammas nchammas mannequin commented Feb 1, 2021

BPO 43094
Nosy @berkerpeksag, @serhiy-storchaka, @nchammas, @erlend-aasland
PRs
  • #24489
  • 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 = None
    created_at = <Date 2021-02-01.20:10:10.173>
    labels = ['3.9', '3.10', 'docs']
    title = 'sqlite3 signature discrepancies between documentation and implementation'
    updated_at = <Date 2021-03-28.21:07:22.364>
    user = 'https://github.com/nchammas'

    bugs.python.org fields:

    activity = <Date 2021-03-28.21:07:22.364>
    actor = 'erlendaasland'
    assignee = 'docs@python'
    closed = False
    closed_date = None
    closer = None
    components = ['Documentation']
    creation = <Date 2021-02-01.20:10:10.173>
    creator = 'nchammas'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 43094
    keywords = ['patch']
    message_count = 10.0
    messages = ['386100', '386120', '386708', '386741', '386806', '386813', '386826', '389544', '389552', '389651']
    nosy_count = 5.0
    nosy_names = ['docs@python', 'berker.peksag', 'serhiy.storchaka', 'nchammas', 'erlendaasland']
    pr_nums = ['24489']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue43094'
    versions = ['Python 3.9', 'Python 3.10']

    @nchammas
    Copy link
    Mannequin Author

    @nchammas nchammas mannequin commented Feb 1, 2021

    The doc for sqlite3.create_function shows the signature as follows:

    https://docs.python.org/3.9/library/sqlite3.html#sqlite3.Connection.create_function

    create_function(name, num_params, func, *, deterministic=False)
    

    But it appears that the parameter name is narg, not num_params. Trying num_params yields:

    TypeError: function missing required argument 'narg' (pos 2)
    

    @nchammas nchammas mannequin added the 3.9 label Feb 1, 2021
    @nchammas nchammas mannequin assigned docspython Feb 1, 2021
    @nchammas nchammas mannequin added docs 3.9 labels Feb 1, 2021
    @nchammas nchammas mannequin assigned docspython Feb 1, 2021
    @nchammas nchammas mannequin added the docs label Feb 1, 2021
    @erlend-aasland
    Copy link
    Contributor

    @erlend-aasland erlend-aasland commented Feb 1, 2021

    There's also a discrepancy between the docs and the signature for create_aggregate():

    https://docs.python.org/3.9/library/sqlite3.html#sqlite3.Connection.create_aggregate

    create_aggregate(name, num_params, aggregate_class)

    The second parameter is named n_arg, here with an underscore.

    I'm not sure what's the best fix; fixing the docs or the signatures. If we fix the signatures, we'll at least get a kind of consistency.

    @erlend-aasland
    Copy link
    Contributor

    @erlend-aasland erlend-aasland commented Feb 9, 2021

    Quoting from Victor Stinner's reply on python-dev (https://mail.python.org/archives/list/python-dev@python.org/message/X6SZIVOZ233TLLJV43UQEHMV3ELGP34S/):

    If there are applications relying on the parameter name, it's better
    to trust the Python implementation, rather than the documentation.

    It would be better to make these parameters positional only in the
    first place, but now it's too late for such backward incompatible
    change :-(

    @erlend-aasland
    Copy link
    Contributor

    @erlend-aasland erlend-aasland commented Feb 9, 2021

    Module level function discrepancies:

    register_converter(), register_adapter(), and enable_callback_tracebacks() docstrings were unintentionally altered in recent AC updates. Docstrings should be restored. Docs are ok.

    adapt() is not documented, but docstrings were unintentionally altered in recent AC updates. Docstrings should be restored.

    connect() and enable_shared_cache() (latter is undocumented) are ok.

    complete_statement() docs should be updated to reflect implementation.

    @erlend-aasland erlend-aasland changed the title sqlite3.create_function takes parameter named narg, not num_params Update sqlite3 docs and docstrings to reflect implementation Feb 9, 2021
    @erlend-aasland erlend-aasland changed the title sqlite3.create_function takes parameter named narg, not num_params Update sqlite3 docs and docstrings to reflect implementation Feb 9, 2021
    @erlend-aasland
    Copy link
    Contributor

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

    sqlite3.Connection.set_progress_handler()

    docs: set_progress_handler(handler, n)
    impl: set_progress_handler(progress_handler, n)

    Apart from that, the rest of sqlite3.Connection seems to be ok.

    There's an ongoing discussion at python-dev about how to resolve this issue. I'm in favour of normalising the create_*() methods to be positional only (like create_collation() is now).

    sqlite3.Cursor and sqlite3.Row methods seems to be ok as well.

    @erlend-aasland erlend-aasland changed the title Update sqlite3 docs and docstrings to reflect implementation sqlite3 signature discrepancies between documentation and implementation Feb 10, 2021
    @erlend-aasland erlend-aasland changed the title Update sqlite3 docs and docstrings to reflect implementation sqlite3 signature discrepancies between documentation and implementation Feb 10, 2021
    @berkerpeksag
    Copy link
    Member

    @berkerpeksag berkerpeksag commented Feb 10, 2021

    The problem is that sqlite3 isn't the only module where there are discrepancies between documentation and implementation. If we are going to change public sqlite3 APIs in to be positional-only, I'd prefer writing a PEP and fix all modules once and for all.

    @erlend-aasland
    Copy link
    Contributor

    @erlend-aasland erlend-aasland commented Feb 11, 2021

    If we are going to change public sqlite3 APIs in to be positional-only, I'd prefer writing a PEP and fix all modules once and for all.

    Right. I assume you mean that such a PEP would contain a set of guidelines for how to handle these dilemmas? It would be almost impossible to create a set of strict rules that can handle all cases, AFAICS.

    @erlend-aasland
    Copy link
    Contributor

    @erlend-aasland erlend-aasland commented Mar 26, 2021

    I'd prefer writing a PEP and fix all modules once and for all.

    Berker, I started drafting a PEP. Would you be interested in helping?

    @berkerpeksag
    Copy link
    Member

    @berkerpeksag berkerpeksag commented Mar 26, 2021

    Berker, I started drafting a PEP. Would you be interested in helping?

    Of course! I may be not be able to respond to you quickly in the next few weeks, though.

    @erlend-aasland
    Copy link
    Contributor

    @erlend-aasland erlend-aasland commented Mar 28, 2021

    Of course! I may be not be able to respond to you quickly in the next few weeks, though.

    Thank you! I'll send you a preliminary version sometime this week.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    erlend-aasland added a commit to erlend-aasland/cpython that referenced this issue Jun 15, 2022
    @erlend-aasland
    Copy link
    Contributor

    @erlend-aasland erlend-aasland commented Jun 15, 2022

    The problem is that sqlite3 isn't the only module where there are discrepancies between documentation and implementation.

    This does not stand in the way of fixing the docs right away.

    If we are going to change public sqlite3 APIs in to be positional-only, I'd prefer writing a PEP and fix all modules once and for all.

    I think that is too large for a PEP. We could create an issue for making some sqlite3 methods positional only. See also gh-93057.

    erlend-aasland added a commit that referenced this issue Jun 15, 2022
    …ion (#93840)
    
    Align the docs for the following methods with the actual implementation:
    
    - sqlite3.complete_statement()
    - sqlite3.Connection.create_function()
    - sqlite3.Connection.create_aggregate()
    - sqlite3.Connection.set_progress_handler()
    miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jun 15, 2022
    …mentation (pythonGH-93840)
    
    Align the docs for the following methods with the actual implementation:
    
    - sqlite3.complete_statement()
    - sqlite3.Connection.create_function()
    - sqlite3.Connection.create_aggregate()
    - sqlite3.Connection.set_progress_handler()
    (cherry picked from commit d318346)
    
    Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@protonmail.com>
    miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jun 15, 2022
    …mentation (pythonGH-93840)
    
    Align the docs for the following methods with the actual implementation:
    
    - sqlite3.complete_statement()
    - sqlite3.Connection.create_function()
    - sqlite3.Connection.create_aggregate()
    - sqlite3.Connection.set_progress_handler()
    (cherry picked from commit d318346)
    
    Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@protonmail.com>
    @erlend-aasland
    Copy link
    Contributor

    @erlend-aasland erlend-aasland commented Jun 15, 2022

    Thanks for the report. If there are further discrepancies, please open a new issue.

    For making sqlite3 methods positional only, please open a new issue.

    miss-islington added a commit that referenced this issue Jun 15, 2022
    …ion (GH-93840)
    
    Align the docs for the following methods with the actual implementation:
    
    - sqlite3.complete_statement()
    - sqlite3.Connection.create_function()
    - sqlite3.Connection.create_aggregate()
    - sqlite3.Connection.set_progress_handler()
    (cherry picked from commit d318346)
    
    Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@protonmail.com>
    miss-islington added a commit that referenced this issue Jun 15, 2022
    …ion (GH-93840)
    
    Align the docs for the following methods with the actual implementation:
    
    - sqlite3.complete_statement()
    - sqlite3.Connection.create_function()
    - sqlite3.Connection.create_aggregate()
    - sqlite3.Connection.set_progress_handler()
    (cherry picked from commit d318346)
    
    Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@protonmail.com>
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.9 3.10 docs expert-sqlite3
    Projects
    Status: Done
    Development

    Successfully merging a pull request may close this issue.

    2 participants