The Wayback Machine - https://web.archive.org/web/20221223191307/https://github.com/python/cpython/issues/76963
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

memoryview & ctypes: incorrect itemsize for empty array #76963

Closed
eric-wieser mannequin opened this issue Feb 6, 2018 · 12 comments · Fixed by #5576
Closed

memoryview & ctypes: incorrect itemsize for empty array #76963

eric-wieser mannequin opened this issue Feb 6, 2018 · 12 comments · Fixed by #5576
Labels
3.8 3.9 3.10 expert-ctypes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@eric-wieser
Copy link
Mannequin

eric-wieser mannequin commented Feb 6, 2018

BPO 32782
Nosy @terryjreedy, @amauryfa, @abalkin, @skrah, @meadori, @eric-wieser
PRs
  • bpo-32782: PEP3118 itemsize of an empty ctypes array should not be 0 #5576
  • 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 2018-02-06.18:53:21.025>
    labels = ['interpreter-core', 'type-bug', '3.8', '3.9', '3.10', 'ctypes']
    title = 'memoryview & ctypes: incorrect itemsize for empty array'
    updated_at = <Date 2020-05-23.15:47:49.464>
    user = 'https://github.com/eric-wieser'

    bugs.python.org fields:

    activity = <Date 2020-05-23.15:47:49.464>
    actor = 'cheryl.sabella'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Interpreter Core', 'ctypes']
    creation = <Date 2018-02-06.18:53:21.025>
    creator = 'Eric.Wieser'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 32782
    keywords = ['patch']
    message_count = 9.0
    messages = ['311740', '321290', '324705', '340230', '340231', '340232', '340235', '340236', '348004']
    nosy_count = 8.0
    nosy_names = ['terry.reedy', 'teoliphant', 'amaury.forgeotdarc', 'belopolsky', 'skrah', 'meador.inge', 'Eric.Wieser', 'Eric Wieser']
    pr_nums = ['5576']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue32782'
    versions = ['Python 3.8', 'Python 3.9', 'Python 3.10']

    Linked PRs

    @eric-wieser
    Copy link
    Mannequin Author

    eric-wieser mannequin commented Feb 6, 2018

    Take the following simple structure:

        class Foo(ctypes.Structure):
            _fields_ = [('f', ctypes.uint32_t)]

    And construct some arrays with it:

        def get_array_view(N):
            return memoryview((Foo * N)())

    In most cases, this works as expected, returning the size of one item:

        >>> get_array_view(10).itemsize
        4
        >>> get_array_view(1).itemsize
        4

    But when N=0, it returns the wrong result

        >>> get_array_view(0).itemsize
        0

    Which contradicts its .format, which still describes a 4-byte struct

        >>> get_array_view(0).format
        'T{>I:one:}'

    This causes a downstream problem in numpy:

        >>> np.array(get_array_view(0))
        RuntimeWarning: Item size computed from the PEP 3118 buffer format string does not match the actual item size.

    @eric-wieser eric-wieser mannequin added expert-ctypes type-bug An unexpected behavior, bug, or error labels Feb 6, 2018
    @eric-wieser
    Copy link
    Mannequin Author

    eric-wieser mannequin commented Jul 8, 2018

    Pinging, as recommended by https://devguide.python.org/pullrequest/#reviewing. Ideally this and https://bugs.python.org/issue32780 would make the same patch release.

    @eric-wieser
    Copy link
    Mannequin Author

    eric-wieser mannequin commented Sep 6, 2018

    Pinging again, for lack of a clearer path forward

    @terryjreedy
    Copy link
    Member

    terryjreedy commented Apr 14, 2019

    This issue is about the itemsize attribute of instances of the built-in memoryview class. Ctypes in only involved in providing format information. Hence the nosy additions.

    On Win10 with 3.8, ctypes has no uint attributes. Using 'c_int32' instead, I see the same behavior (itemsize 0 for empty structure).

    About itemsize, https://www.python.org/dev/peps/pep-3118/ says

    "This is a storage for the itemsize (in bytes) of each element of the shared memory. It is technically un-necessary as it can be obtained using PyBuffer_SizeFromFormat, however an exporter may know this information without parsing the format string and it is necessary to know the itemsize for proper interpretation of striding. Therefore, storing it is more convenient and faster."

    The first line could be seen as implying that itemsize is undefined if there are no items (and as justifying numbytes/numitems otherwise). The 0 return could be seen as equivalent to a None return from a python-coded function. If so, it is not a bug, and there might be code that would break if it is changed.

    On the other hand, the next lines imply that itemsize is *usually*, though not necessarily, a cache for PyBuffer_SizeFromFormat. This could be seen as implying that in the absence of other information, the itemsize should be calculated from the format, making 0 a bug.

    @terryjreedy terryjreedy added interpreter-core (Objects, Python, Grammar, and Parser dirs) 3.8 and removed expert-ctypes labels Apr 14, 2019
    @terryjreedy terryjreedy changed the title ctypes: memoryview gives incorrect PEP3118 itemsize for array of length zero memoryview gives incorrect PEP3118 itemsize for empty array Apr 14, 2019
    @terryjreedy
    Copy link
    Member

    terryjreedy commented Apr 14, 2019

    This is actually about memoryview.itemsize within ctypes.

    @terryjreedy terryjreedy changed the title memoryview gives incorrect PEP3118 itemsize for empty array memoryview & ctypes: incorrect PEP3118 itemsize for empty array Apr 14, 2019
    @terryjreedy
    Copy link
    Member

    terryjreedy commented Apr 14, 2019

    https://docs.python.org/3/library/stdtypes.html#typememoryview
    says "itemsize The size in bytes of each element of the memoryview"
    Revising the code example to use an empty array:
    >>> import array, struct
    >>> m = memoryview(array.array('H', [0])
    >>> m.itemsize
    2
    I agree that itemsize should also be non-zero for ctype formats.

    @terryjreedy terryjreedy changed the title memoryview & ctypes: incorrect PEP3118 itemsize for empty array memoryview & ctypes: incorrect itemsize for empty array Apr 14, 2019
    @skrah
    Copy link
    Mannequin

    skrah mannequin commented Apr 14, 2019

    I agree that it is a ctypes issue, itemsize should be equal to struct.calcsize(fmt), which is never 0 for normal PEP-3118 types like the one in the example.

    [Pedantically, I think that the grammar would allow for an empty record "T{}" that would have itemsize 0 but is of little use inside numpy.]

    @EricWieser
    Copy link
    Mannequin

    EricWieser mannequin commented Apr 14, 2019

    Revising the code example to use an empty array

    I think you mean

    >>> import array
    >>> memoryview(array.array('H', [])).itemsize
    2

    Your example is an array containing 0, not an empty array - but the conclusion is the same.

    It is technically un-necessary as it can be obtained using PyBuffer_SizeFromFormat

    This obviously predicates on PyBuffer_SizeFromFormat being implemented, which according to the docs it is not.

    I think that the grammar would allow for an empty record "T{}" that would have itemsize 0 but is of little use inside numpy.

    It also allows for records of empty arrays, "T{(0)d:data:}". While these are of little use, they _are_ supported by both ctypes and numpy, so we should support them in the PEP-3118 interface used between them.

    @EricWieser
    Copy link
    Mannequin

    EricWieser mannequin commented Jul 16, 2019

    Pinging again, now that the patch has undergone a revision with some cleanup thanks to @skrah

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    mdickinson pushed a commit that referenced this issue Dec 23, 2022
    …H-5576)
    
    The itemsize returned in a memoryview of a ctypes array is now computed from the item type, instead of dividing the total size by the length and assuming that the length is not zero.
    miss-islington pushed a commit to miss-islington/cpython that referenced this issue Dec 23, 2022
    …be 0 (pythonGH-5576)
    
    The itemsize returned in a memoryview of a ctypes array is now computed from the item type, instead of dividing the total size by the length and assuming that the length is not zero.
    (cherry picked from commit 84bc6a4)
    
    Co-authored-by: Eric Wieser <[email protected]>
    miss-islington pushed a commit to miss-islington/cpython that referenced this issue Dec 23, 2022
    …be 0 (pythonGH-5576)
    
    The itemsize returned in a memoryview of a ctypes array is now computed from the item type, instead of dividing the total size by the length and assuming that the length is not zero.
    (cherry picked from commit 84bc6a4)
    
    Co-authored-by: Eric Wieser <[email protected]>
    mdickinson pushed a commit that referenced this issue Dec 23, 2022
    … be 0 (GH-5576) (#100451)
    
    gh-76963: PEP3118 itemsize of an empty ctypes array should not be 0 (GH-5576)
    
    The itemsize returned in a memoryview of a ctypes array is now computed from the item type, instead of dividing the total size by the length and assuming that the length is not zero.
    (cherry picked from commit 84bc6a4)
    
    Co-authored-by: Eric Wieser <[email protected]>
    mdickinson pushed a commit that referenced this issue Dec 23, 2022
    … be 0 (GH-5576) (GH-100452)
    
    gh-76963: PEP3118 itemsize of an empty ctypes array should not be 0 (GH-5576)
    
    The itemsize returned in a memoryview of a ctypes array is now computed from the item type, instead of dividing the total size by the length and assuming that the length is not zero.
    (cherry picked from commit 84bc6a4)
    
    Co-authored-by: Eric Wieser <[email protected]>
    @mattip
    Copy link
    Contributor

    mattip commented Dec 23, 2022

    Thanks for the fix that closed this. @eric-wieser do you remember where this impacted NumPy? I don't see the relevant issue or PR.

    @eric-wieser
    Copy link
    Contributor

    eric-wieser commented Dec 23, 2022

    I think the workaround in numpy needs to stay till after #5561

    @eric-wieser
    Copy link
    Contributor

    eric-wieser commented Dec 23, 2022

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.8 3.9 3.10 expert-ctypes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    Successfully merging a pull request may close this issue.

    4 participants