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

Support __add__, __mul__, and __imul__ for deques. #67981

Closed
rhettinger opened this issue Mar 27, 2015 · 8 comments
Closed

Support __add__, __mul__, and __imul__ for deques. #67981

rhettinger opened this issue Mar 27, 2015 · 8 comments
Assignees
Labels
extension-modules C modules in the Modules dir type-feature A feature request or enhancement

Comments

@rhettinger
Copy link
Contributor

BPO 23793
Nosy @rhettinger, @MojoVampire
Files
  • deque_add4.diff: Patch with tests
  • deque_add5.diff: Restrict addition to objects of the same type
  • deque_mul_and_add.diff: add, mul, and imul
  • 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/rhettinger'
    closed_at = <Date 2015-03-31.15:14:17.881>
    created_at = <Date 2015-03-27.17:33:58.600>
    labels = ['extension-modules', 'type-feature']
    title = 'Support __add__, __mul__, and __imul__ for deques.'
    updated_at = <Date 2015-03-31.15:14:17.881>
    user = 'https://github.com/rhettinger'

    bugs.python.org fields:

    activity = <Date 2015-03-31.15:14:17.881>
    actor = 'rhettinger'
    assignee = 'rhettinger'
    closed = True
    closed_date = <Date 2015-03-31.15:14:17.881>
    closer = 'rhettinger'
    components = ['Extension Modules']
    creation = <Date 2015-03-27.17:33:58.600>
    creator = 'rhettinger'
    dependencies = []
    files = ['38712', '38713', '38734']
    hgrepos = []
    issue_num = 23793
    keywords = ['needs review']
    message_count = 8.0
    messages = ['239419', '239563', '239642', '239646', '239652', '239682', '239722', '239724']
    nosy_count = 3.0
    nosy_names = ['rhettinger', 'python-dev', 'josh.r']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'patch review'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue23793'
    versions = ['Python 3.5']

    @rhettinger
    Copy link
    Contributor Author

    One more step towards making the deque API a little closer to that for lists:
      
        >>> deque('abc') + deque('def')
        deque(['a', 'b', 'c', 'd', 'e', 'f'])

    @rhettinger rhettinger self-assigned this Mar 27, 2015
    @rhettinger rhettinger added extension-modules C modules in the Modules dir type-feature A feature request or enhancement labels Mar 27, 2015
    @rhettinger
    Copy link
    Contributor Author

    Also incorporate __mul__ and __imul__.

        >>> d = deque('abc')
        >>> d * 3
        deque(['a', 'b', 'c', 'a', 'b', 'c', 'a', 'b', 'c'])
        >>> d *= 2
        >>> d
        deque(['a', 'b', 'c', 'a', 'b', 'c'])

    @rhettinger rhettinger changed the title Support __add__ for deques. Support __add__, __mul__, and __imul__ for deques. Mar 30, 2015
    @MojoVampire
    Copy link
    Mannequin

    MojoVampire mannequin commented Mar 30, 2015

    The behavior for multiplying or adding doesn't seem quite so intuitive when you allow for a bounded deque. In particular, it doesn't seem obvious that multiplying when the deque is bounded, you'll get results like:

    >>> list(deque([1,2], 3) * 2)
    [2, 1, 2]

    Similarly, when you support non-in-place addition, the bounded-ness of the result depends on the order of the values added.

    >>> list(deque([1,2], 3) + deque([1,2], 2))
    [1, 2, 1]
    >>> list(deque([1,2], 2) + deque([1,2], 3))
    [1, 2]

    Not saying these are the wrong behaviors, but it's much more weird than what you get with other sequence types (since most sequence types aren't bounded), and possibly why deques didn't include these features in the first place; trying to act like a generalized sequence when you have features that don't fit the model is a titch odd.

    @MojoVampire
    Copy link
    Mannequin

    MojoVampire mannequin commented Mar 30, 2015

    I think my first addition example is wrong (should produce [2, 1, 2]), but you get the idea.

    @rhettinger
    Copy link
    Contributor Author

    The behavior for multiplying or adding doesn't seem quite so
    intuitive when you allow for a bounded deque.

    What would you want it to do? By design, the key feature of maxlen is pop old inputs to make way newer appends -- that is its essence.

    It would be surprising if the following invariant didn't hold:

    >>> deque('abc' * 3, maxlen=5) == deque('abc', maxlen=5) * 3
    True

    That said, I don't expect that people are going to commonly be doing d*=n where len(d) > 1 and there is a maxlen > len(d)*n. The normal cases are unsurprising.

    @MojoVampire
    Copy link
    Mannequin

    MojoVampire mannequin commented Mar 31, 2015

    I agree that popping old inputs is the normal way. What I'm trying to say is that it doesn't feel like "old" or "inputs" when you multiply. In that case (and maybe this is just me), it feels like a "reasonable" outcome could be to get an outcome similar to multiplying a list and then slicing the result to maxlen. I don't think it *should* do that, but either behavior feels weird, solely because sequence multiplication has never been applied to a bounded sequence before (to my knowledge), so the expectations aren't set.

    You end up with weird behaviors like len(seq) * 3 != len(seq * 3), which is a normal and expected outcome with every other sequence. If you know you're dealing with a bounded deque, it's less unexpected, but this is trying to make a deque a drop-in replacement for any other sequence when it doesn't behave like one.

    @rhettinger
    Copy link
    Contributor Author

    I don't see any technical reason not to proceed. Just because an unusual combination of parameters feels odd to you doesn't make it wrong. It is no different than (somelist * n)[-maxlen:].

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 31, 2015

    New changeset b75160d24b7b by Raymond Hettinger in branch 'default':
    bpo-23793: Add deque support for __add__(), __mul__(), and __imul__().
    https://hg.python.org/cpython/rev/b75160d24b7b

    @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
    extension-modules C modules in the Modules dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant