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

Allow reversed(memoryview), like memoryview #63278

Closed
PCManticore mannequin opened this issue Sep 23, 2013 · 12 comments
Closed

Allow reversed(memoryview), like memoryview #63278

PCManticore mannequin opened this issue Sep 23, 2013 · 12 comments
Assignees
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@PCManticore
Copy link
Mannequin

PCManticore mannequin commented Sep 23, 2013

BPO 19078
Nosy @rhettinger, @ncoghlan, @pitrou, @skrah, @PCManticore
Files
  • memoryview.patch
  • 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 = 'https://github.com/ncoghlan'
    closed_at = <Date 2013-10-02.12:07:06.836>
    created_at = <Date 2013-09-23.16:27:16.275>
    labels = ['interpreter-core', 'type-feature']
    title = 'Allow reversed(memoryview), like memoryview'
    updated_at = <Date 2013-10-02.12:07:06.834>
    user = 'https://github.com/PCManticore'

    bugs.python.org fields:

    activity = <Date 2013-10-02.12:07:06.834>
    actor = 'python-dev'
    assignee = 'ncoghlan'
    closed = True
    closed_date = <Date 2013-10-02.12:07:06.836>
    closer = 'python-dev'
    components = ['Interpreter Core']
    creation = <Date 2013-09-23.16:27:16.275>
    creator = 'Claudiu.Popa'
    dependencies = []
    files = ['31850']
    hgrepos = []
    issue_num = 19078
    keywords = ['patch']
    message_count = 12.0
    messages = ['198324', '198342', '198348', '198383', '198384', '198516', '198518', '198566', '198612', '198680', '198824', '198829']
    nosy_count = 6.0
    nosy_names = ['rhettinger', 'ncoghlan', 'pitrou', 'skrah', 'Claudiu.Popa', 'python-dev']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue19078'
    versions = ['Python 3.4']

    @PCManticore
    Copy link
    Mannequin Author

    PCManticore mannequin commented Sep 23, 2013

    Hello. The following seems a little weird:

    Python 3.3.2 (v3.3.2:d047928ae3f6, May 16 2013, 00:03:43) [MSC v.1600 32 bit (Intel)] on win32
    >>> m = memoryview(b'123')
    >>> list(m[::-1])
    [51, 50, 49]
    >>> list(reversed(m))
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    TypeError: object of type 'memoryview' has no len()
    >>>

    The attached patch allows reversed to be called with memoryviews and it could potentially fix bpo-18690.

    @PCManticore PCManticore mannequin added type-bug An unexpected behavior, bug, or error interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Sep 23, 2013
    @pitrou
    Copy link
    Member

    pitrou commented Sep 23, 2013

    So the dilemma with len() was: does it return the number of bytes, or the number of items?
    Given the new memoryview semantics, I'd say it should return the number of items.

    @ncoghlan
    Copy link
    Contributor

    Aye, with the 3.3 changes, I think we should continue down the
    multi-dimensional array path. I suggest we run with whatever NumPy uses for
    ndarray, which I believe is "number of items in the first dimension".

    @ncoghlan ncoghlan changed the title Allow reversed(memoryview), like memoryview[::-1] Allow reversed(memoryview), like memoryview Sep 23, 2013
    @PCManticore
    Copy link
    Mannequin Author

    PCManticore mannequin commented Sep 25, 2013

    So, in the following case, len shouldn't return 4, but the number of items? Also, is it ok to assume that the number of items is
    ((*shape)[0] * ... * (*shape)[ndims-1])?

    >>> x = np.array([[1, 2, 3], [4, 5, 6], [4,5,6], [4,4,4]], np.int32)
    >>> x.shape
    (4, 3)
    >>> x.shape[0] * x.shape[1]
    12
    >>> len(x)
    4
    >>> memoryview(x)
    <memory at 0x0217C378>
    >>> len(memoryview(x))
    4
    >>> memoryview(x).shape
    (4, 3)
    >>>

    @pitrou
    Copy link
    Member

    pitrou commented Sep 25, 2013

    No, you're right, it should probably return 4.

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented Sep 28, 2013

    Yes, len() should return the number of items. +1 for making reversed()
    work.

    NumPy does this:

    >>> x = numpy.array([1,2,3,4,5,6,7,8])
    >>> list(reversed(x))
    [8, 7, 6, 5, 4, 3, 2, 1]
    >>>
    >>> x = numpy.array([[1,2,3], [4,5,6]])
    >>> list(reversed(x))
    [array([4, 5, 6]), array([1, 2, 3])]
    >>>

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented Sep 28, 2013

    Hmm, I meant: Number of items in the first dimension, thus 4 in
    Claudiu's example.

    @terryjreedy terryjreedy added type-feature A feature request or enhancement and removed type-bug An unexpected behavior, bug, or error labels Sep 28, 2013
    @PCManticore
    Copy link
    Mannequin Author

    PCManticore mannequin commented Sep 29, 2013

    For multidimensional arrays it doesn't seem to work (yet).

    >>> x = numpy.array([[1,2,3], [4,5,6]])
    >>> list(reversed(x))
    [array([4, 5, 6]), array([1, 2, 3])]
    >>> x.data
    <memory at 0x8032d06b8>
    >>> list(reversed(x.data))
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    NotImplementedError: multi-dimensional sub-views are not implemented
    >>>

    @pitrou
    Copy link
    Member

    pitrou commented Sep 29, 2013

    Stefan, what do you think about Claudiu's patch? Should a test be added to test_buffer as well?

    @rhettinger
    Copy link
    Contributor

    Claudiu's patch looks correct.

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented Oct 2, 2013

    Stefan, what do you think about Claudiu's patch? Should a test be added to test_buffer as well?

    I think the patch is good. We can add more tests when (if?) multi-dimensional
    support is added to memoryview.

    In that case we should probably do the same as NumPy and return a list of
    subviews. So testing against tolist() like in the test case will only work
    for one-dimensional views.

    I can't commit right now (the machine with my ssh-key won't have Internet
    access for some time), so if someone has time to do it ...

    @ncoghlan ncoghlan self-assigned this Oct 2, 2013
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Oct 2, 2013

    New changeset 0dc604d58949 by Nick Coghlan in branch 'default':
    Close bpo-19078: memoryview now supports reversed
    http://hg.python.org/cpython/rev/0dc604d58949

    @python-dev python-dev mannequin closed this as completed Oct 2, 2013
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants