classification
Title: ctypes string_at/wstring_at - bad argument name used in docs and in docstring
Type: behavior Stage: patch review
Components: ctypes, Documentation Versions: Python 3.11, Python 3.10, Python 3.9
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: docs@python Nosy List: Jelle Zijlstra, amaury.forgeotdarc, belopolsky, docs@python, eryksun, meador.inge, shreyanavigyan, talhayon1
Priority: normal Keywords: patch

Created on 2021-04-11 01:54 by talhayon1, last changed 2021-06-14 10:20 by iritkatriel.

Pull Requests
URL Status Linked Edit
PR 25384 open shreyanavigyan, 2021-04-13 09:36
Messages (11)
msg390761 - (view) Author: Tal Hayon (talhayon1) Date: 2021-04-11 01:54
string_at and wstring_at first argument in docs in address but actually in code is ptr.

See
https://github.com/python/cpython/blob/750f484752763fe9ac1d6455780aabcb67f25508/Lib/ctypes/__init__.py#L513
https://github.com/python/cpython/blob/750f484752763fe9ac1d6455780aabcb67f25508/Lib/ctypes/__init__.py#L525

Noticed this when going over stubtest errors in typeshed.
See pull request https://github.com/python/typeshed/pull/5204
msg390762 - (view) Author: Jelle Zijlstra (Jelle Zijlstra) * (Python triager) Date: 2021-04-11 01:59
It's a bit worse: the actual name is "ptr", the function docstrings say "addr", and the documentation (https://docs.python.org/3.9/library/ctypes.html#ctypes.string_at) has "address". I'd favor updating them all to say "ptr", because changing the name of the runtime parameter would risk breaking users' code.
msg390791 - (view) Author: Shreyan Avigyan (shreyanavigyan) * Date: 2021-04-11 19:19
> I'd favor updating them all to say "ptr", because changing the name of the runtime parameter would risk breaking users' code.

I also certainly agree with that. Moreover, the documentation and docstring uses the name "address" and "addr" respectively which are misleading because it is asking for ctypes.c_char_p or ctypes.c_wchar_p which are C pointer types, it is not asking for the address of instances of ctypes.c_char_p or ctypes.c_wchar_p. Therefore a change in the documentation and docstring is required.

Note - If this issue is confirmed then I'll submit a PR to apply the change as described in this issue.
msg390795 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2021-04-11 21:16
> the name "address" and "addr" respectively which are misleading 
> because it is asking for ctypes.c_char_p or ctypes.c_wchar_p which 
> are C pointer types

string_at() and wstring_at() take a c_void_p value. This can be initialized by an integer (i.e. an address), bytes, str, or any ctypes pointer, array, or byref() reference. 

Additionally it works with an object that has a compatible _as_parameter_ attribute, e.g.:

    >>> class A:
    ...    _as_parameter_ = b'spam'
    ... 
    >>> ctypes.string_at(A)
    b'spam'

If the docstring is change to use "ptr", for string_at() it can succinctly say the following: "Return the byte string at void *ptr." 

For wstring_at(), replace "byte string" with "wide-character string" or "wchar_t string". Specifying the type matters because it depends on the platform. Windows uses a 16-bit wchar_t, and the memory will be interpreted as UTF-16 (e.g. surrogate pairs), whereas other platforms use a 32-bit wchar_t, and the memory will be interpreted as UTF-32. For example:

Windows:

    >>> ascii(ctypes.wstring_at(b'\x00\xd8\x00\xdc'))
    "'\\U00010000'"

Linux:

    >>> try: ctypes.wstring_at(b'\x00\xd8\x00\xdc')
    ... except Exception as e: print(e)
    ... 
    character U+dc00d800 is not in range [U+0000; U+10ffff]
msg390885 - (view) Author: Shreyan Avigyan (shreyanavigyan) * Date: 2021-04-12 18:40
Yeah, you're right and also it is true that changing the docstring to "ptr" can succinctly say "Return the byte string at void *ptr." But it's a kind of problem if different argument names are used for the same argument in docstring, argument list and documentation. If that's the problem then why not change all of them to use the name "address" or "addr" to avoid confusion and make it clear simple. What is your view?
msg390912 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2021-04-12 21:56
> If that's the problem then why not change all of them to use the 
> name "address" or "addr" to avoid confusion and make it clear 
> simple. What is your view?

Change them all to refer to `ptr`. It's not worth introducing a breaking change:

    >>> ctypes.string_at(ptr=b'spam')
    b'spam'
msg390925 - (view) Author: Shreyan Avigyan (shreyanavigyan) * Date: 2021-04-13 06:10
Ok...let me review again. Having different names for the same argument can cause confusion, right? So are you telling that we should change the name to "ptr" to avoid a breaking change?

If this issue is confirmed, I'm ready to submit a PR to make this change.
msg390928 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2021-04-13 06:50
> So are you telling that we should change the name to "ptr" to 
> avoid a breaking change?

The function parameter name is "ptr". Changing that name could break existing code. I don't think it's likely, but it's not worth breaking even one script just for this. So change the docstring and documentation to refer to "ptr".
msg390929 - (view) Author: Shreyan Avigyan (shreyanavigyan) * Date: 2021-04-13 06:51
Ok..then submitting the PR in 10 minutes.
msg390943 - (view) Author: Shreyan Avigyan (shreyanavigyan) * Date: 2021-04-13 09:37
PR opened for this issue.
msg391939 - (view) Author: Shreyan Avigyan (shreyanavigyan) * Date: 2021-04-26 14:19
Ping
History
Date User Action Args
2021-06-14 10:20:50iritkatrielsetversions: + Python 3.11, - Python 3.8
2021-04-26 14:19:01shreyanavigyansetmessages: + msg391939
2021-04-19 09:03:51shreyanavigyansetnosy: + amaury.forgeotdarc, belopolsky, meador.inge
2021-04-16 20:21:20terry.reedysetversions: - Python 3.6, Python 3.7
2021-04-13 09:37:31shreyanavigyansetmessages: + msg390943
2021-04-13 09:36:19shreyanavigyansetkeywords: + patch
stage: patch review
pull_requests: + pull_request24116
2021-04-13 06:51:48shreyanavigyansetmessages: + msg390929
2021-04-13 06:50:28eryksunsetmessages: + msg390928
2021-04-13 06:10:25shreyanavigyansetmessages: + msg390925
2021-04-12 21:56:43eryksunsetmessages: + msg390912
2021-04-12 18:40:31shreyanavigyansetmessages: + msg390885
2021-04-11 21:16:08eryksunsetnosy: + eryksun
messages: + msg390795
2021-04-11 19:19:38shreyanavigyansettitle: ctypes string_at/wstring_at bad argument name -> ctypes string_at/wstring_at - bad argument name used in docs and in docstring
nosy: + shreyanavigyan, docs@python

messages: + msg390791

assignee: docs@python
components: + Documentation
2021-04-11 01:59:52Jelle Zijlstrasetnosy: + Jelle Zijlstra
messages: + msg390762
2021-04-11 01:55:15talhayon1settitle: ctypes string_at/wstring_at -> ctypes string_at/wstring_at bad argument name
2021-04-11 01:54:33talhayon1create