Skip to content

Add "key" argument to "bisect" module functions #48606

Description

@tebeka
mannequin
BPO 4356
Nosy @rhettinger, @terryjreedy, @gpshead, @tebeka, @mdickinson, @alex, @vadmium, @dimaqq, @1st1, @NeilGirdhar, @MojoVampire, @wumpus, @pablogsal, @miss-islington, @remilapeyre, @GPery
PRs
  • bpo-4356: Add key parameter to functions in bisect module #11781
  • bpo-4356: Add key parameter to functions in bisect module #11781
  • bpo-4356: Add key function support to the bisect module #20556
  • bpo-4356: Mention the new key arguments for the bisect module APIs in the 3.10 What's new #28339
  • [3.10] bpo-4356: Mention the new key arguments for the bisect module APIs in the 3.10 What's new (GH-28339) #28340
  • Files
  • bisect-trunk.patch: bisect for trunk
  • bisect-py3k.patch: patch for py3k
  • bench_bisect_key.py: benchmark
  • SortedCollection.py: Draft for a Sorted Collection class
  • 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 2020-10-20.05:04:39.386>
    created_at = <Date 2008-11-19.17:17:27.762>
    labels = ['type-feature', 'library', '3.10']
    title = 'Add "key" argument to "bisect" module functions'
    updated_at = <Date 2021-10-04.19:18:43.811>
    user = 'https://github.com/tebeka'

    bugs.python.org fields:

    activity = <Date 2021-10-04.19:18:43.811>
    actor = 'pablogsal'
    assignee = 'rhettinger'
    closed = True
    closed_date = <Date 2020-10-20.05:04:39.386>
    closer = 'rhettinger'
    components = ['Library (Lib)']
    creation = <Date 2008-11-19.17:17:27.762>
    creator = 'tebeka'
    dependencies = []
    files = ['15248', '15249', '15250', '16944']
    hgrepos = []
    issue_num = 4356
    keywords = ['patch']
    message_count = 43.0
    messages = ['76060', '76073', '76084', '76092', '76098', '76099', '76203', '76230', '94836', '103143', '103145', '103155', '103158', '103180', '103294', '113222', '115176', '115189', '115288', '152967', '152969', '152983', '208020', '237408', '242267', '246371', '246384', '261007', '305518', '335841', '337737', '337741', '343529', '343544', '343548', '343549', '345220', '345227', '345228', '379086', '401796', '401798', '403173']
    nosy_count = 22.0
    nosy_names = ['rhettinger', 'terry.reedy', 'gregory.p.smith', 'jafo', 'tebeka', 'mark.dickinson', 'alex', 'milko.krachounov', 'dmtr', 'bls', 'martin.panter', 'Dima.Tisnek', 'yselivanov', 'NeilGirdhar', 'josh.r', 'ericreynolds', 'wumpus', 'pablogsal', 'miss-islington', 'remi.lapeyre', 'gpery', 'flyingosprey']
    pr_nums = ['11781', '11781', '20556', '28339', '28340']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue4356'
    versions = ['Python 3.10']

    Activity

    1. tebeka commented on Nov 19, 2008

      tebekamannequin
      MannequinAuthor

      It'd be helpful of the functions in the "bisect" modules will have a
      "key" argument just like "sort".

    2. added
      stdlibStandard Library Python modules in the Lib/ directory
      type-featureA feature request or enhancement
      on Nov 19, 2008
    3. mdickinson commented on Nov 19, 2008

      @mdickinson
      Member

      This request has come up repeatedly (and been rejected) in the past. See issues 2954, 3374, 1185383, 1462228, 1451588, 1619060.

      Could you perhaps explain your particular use case for this? A few truly
      convincing use-cases might increase the chances of this getting accepted.

    4. rhettinger commented on Nov 20, 2008

      @rhettinger
      Contributor

      Miki, the issue is that bisect calls tend to be made repeatedly, so the
      key function can be called over and over again for the same argument.
      It is almost always a better design to simply decorate the list so the
      key function never gets called more than once per element in the list.
      If we added key= to bisect, it would encourage bad design and steer
      people after from better solutions.

    5. mdickinson commented on Nov 20, 2008

      @mdickinson
      Member

      What about cases where performance is unimportant, or where the key
      function is fast (e.g. an attribute access)? Then something like

      bisect(a, x, key=attrgetter('size'))

      is easy to write and read. Mightn't this be considered good design,
      from some perspectives?

      Another thought: if your list is a list of user-defined objects then a
      natural way to do the 'decorate' step of DSU might be to add a 'key'
      attribute to each object, rather than the usual method of constructing
      pairs. (This has the advantage that you might not have to bother with the
      'undecorate' step.) With a key argument, bisect could make use of this
      technique too.

      Disclaimer: I haven't personally had any need for a key argument on
      bisect, so all this is hypothetical. That's why I'm asking for real use-
      cases.

    6. rhettinger commented on Nov 20, 2008

      @rhettinger
      Contributor

      I had said "almost always". Sure, if you don't care about performance
      or scalability, a key= argument would be a net win.

      We're responsible for creating an API that steers most programmers in
      the right direction (Tim sez "we read Knuth so you don't have to").
      Algorithmically, the bisect functions are at the wrong level of
      granularity for applying a key function.

      For user-defined objects, there is no need for a key-attribute since can
      just supply a custom comparison method:

      class UserDefined:
        . . .
        def cmp(self, other):
            return cmp(self.key, other.key)
    7. mdickinson commented on Nov 20, 2008

      @mdickinson
      Member

      One case I've been thinking about is that of maintaining a list of Decimal
      objects that are sorted by absolute value. For this, having to create a
      list of (abs(x), x) pairs just seems clumsy compared to using a key
      argument to bisect.

      Perhaps this is a contrived use case, but it doesn't seem totally
      implausible.

    8. tebeka commented on Nov 21, 2008

      tebekamannequin
      MannequinAuthor

      I agree you can get around this with defining __cmp__, however same goes
      to "sort" and it was added anyway.

      My take on it is that sometimes I need to find something in a big list
      of objects, and I don't like to do DSU and not add __cmp__ to the
      objects (since some of the lists might be sorted by different attributes

      • say one list for time and one line for price).

      I'd prefer if we do implement "key" and add a warning in the docs it
      might slow you down. Which what will happen in the case of __cmp__ anyway.

      I don't see why the "key" function should be called all the time on
      inserted item, it's very easy to cache this value

      def bisect(a, x, lo=0, hi=None, key=lambda x: x):
          assert low >= 0, "low must be non-negative"
          hi = hi or len(a)
      
          x_key = key(x) 
          while lo < hi:
              mid = (lo+hi)//2
              if x_key < key(a[mid]): hi = mid
              else: lo = mid+1
          return lo

      (I'd also wish for "identity" built in, however this is another subject :)

    9. terryjreedy commented on Nov 22, 2008

      @terryjreedy
      Member

      Just a reminder that __cmp__ is gone in 3.0.
      I presume bisect, like sort, only requires __lt__ and perhaps __eq__,
      though I can find no doc of either.

    10. milkokrachounov commented on Nov 2, 2009

      milkokrachounovmannequin
      Mannequin

      I've been bugged by the lack of key= argument for bisect for some time
      now, and today I got to read this and the previous issues about the
      matter. I still fail to understand the reasons for the rejections. It
      might encourage bad design in which expensive key functions will get
      called more than once for the same element. However:

      1. Not all key= functions are computationally expensive, in fact a quick
        search shows that most key= uses come are with itemgetter, attrgetter or
        some cousin.
      2. Even for a computationally expensive function precached key isn't
        necessarily the right answer. And if the keys are huge, a key= function
        can actually be useful in implementing intelligent caching.
      3. The reason for rejection is merely a performance issue which *can* be
        fixed without any significant changes to the design of the app should it
        prove to be a real issue.

      I was writing a module that keeps a sorted list of items in which items
      can be added, removed, or changed (they might get reordered). I used
      bisect to avoid useless slow linear searches, which would be one
      obstacle for the app to scale well. However, the sort key can be changed
      at any time, so implementing __lt__ wasn't an option. Keeping a second
      list with the keys is both memory and computationally expensive as
      inserting into a list is slow. It's not a shiny example of a bisect
      use-case as the only thing I've used my module so far is to display the
      files in a directory, which means the lists are small and the changes
      are seldom, so any implementation is good enough. But bisect with key=
      would have been best if it was available. What's worse, I have a buggy
      unreadable ad hoc implementation of bisect in my code right now. ;)

      I see the following as reasons why key= would provide benefit:

      1. If you have a sorted list, bisect is a net win. Having a key= would
        enable you to utilize it without refactoring anything. The lack of key
        may as well encourage you to continue using linear searches, or other
        sub-optimal solutions.
      2. Using key= is more readable and less error-prone than keeping two
        lists in simple cases where defining a class with __le__ is an overkill.
        Two examples I had where this was the case:
        a) A class I implemented to pick numbers from a certain discrete random
        distribution with bisect. Oh, but yeah, the implementation without key=
        is twice faster.
        b) I gave a hand to someone who was implementing a dictionary looked up
        the words as you typed them with bisect.
      3. Insort. Insort is already slow enough as it is, a key= wouldn't slow
        it down much if at all. In fact, if you are keeping two lists, key=
        would speed it up. Insort with key= is a net win.

      I've implemented key= and quickly hacked a benchmark to show what
      performance hit it might have, and to put things into perspective. The
      results:

      1. Cheap key function, integers and abs(), 1000000 items, 5000 bisects
        or insorts:
        a) Bisect with a key: 0.02205014s
        b) Bisect with a second list: 0.01328707s
        c) Insort with a key: 5.83688211s
        d) Bisect with a second list, and two inserts: 16.17302299s

      2. Cheap key function, ~4000 char bytestrings and len(), 100000 items,
        5000 bisects or insorts:
        a) Bisect with a key: 0.01829195s
        b) Bisect with a second list: 0.00945401s
        c) Insort with a key: 0.25511408s
        d) Bisect with a second list, and two inserts: 0.49303603s

      3. Expensive key function, ~4000 char bytestrings and str.lower(),
        100000 (500 MB) items, 5000 bisects or insorts:
        a) Bisect with a key: 1.26837015s
        b) Bisect with a second list: 0.08390594s
        c) Insort with a key: 1.50406289s
        d) Bisect with a second list, and two inserts: 0.57737398s

      4. Expensive key function, ~4000 char bytestrings and str.lower(),
        500000 (2.5 GB) items, 5000 bisects or insorts:
        a) Bisect with a key: 1.46136308s
        b) Bisect with a second list: 0.08768606s
        c) Insort with a key: 3.05218720s
        d) Bisect with a second list, and two inserts: 6.43227386s

      5. Expensive key function, ~4000 char bytestrings and str.strip(),
        100000 (500 MB) items, 5000 bisects or insorts:
        a) Bisect with a key: 0.03311396s
        b) Bisect with a second list: 0.01374602s
        c) Insort with a key: 0.27423000s
        d) Bisect with a second list, and two inserts: 0.49585080s

      6. Expensive key function, ~4000 char bytestrings and str.strip(),
        500000 (2.5 GB) items, 5000 bisects or insorts:
        a) Bisect with a key: 0.04530501
        b) Bisect with a second list: 0.01912594
        c) Insort with a key: 1.62209797
        d) Bisect with a second list, and two inserts: 5.91734695

      Also, I tried to bench linear searches, but as they had to run in Python
      code they aren't representative of anything. In the integer test they
      went for about 250 seconds without recalculating the key, and for about
      500 with. Also, replacing them with .index() gave about 60 seconds if I
      ensured there's high probability that the number is in the list, and for
      500 if I didn't.

      In short, key= for bisect would be convenient and neat, really useful in
      rare cases, leading to more readable code in the most common cases, and
      I'm unconvinced of the perceived harm that it would cause.

      I attach a patch implementing it for trunk, py3k, and the benchmark
      script I used to test it (on trunk).

    11. self-assigned this
      on Nov 2, 2009
    12. bls commented on Apr 14, 2010

      blsmannequin
      Mannequin

      This was closed over a year ago, but since mark.dickinson was asking for convincing use-cases: I'm breaking up a file into line-delimited chunks. These chunks are non-overlapping, contiguous, and tend to be fairly large, so I'm just recording the start line of each chunk in a 2-ple:

      mapping = [
        (10, 'first chunk'),
        (50, 'second chunk'),
        (60, 'third chunk')
      ]

      Lines 10-49 are in the first chunk, lines 50-59 are in the second, lines 60+ are in the third. So:

      def CategorizeLine(line, mapping):
         loc = bisect.bisect([m[0] for m in mapping], line)
         if loc == 0:
            return None # before first chunk
         return mapping[loc-1][1]

      It Would Be Nice if I could write the second line as:

         loc = bisect.bisect(mapping, line, key=lambda m:m[0])

      The bisect documentation suggests pre-computing the key list, but it seems messy and error-prone to keep a redundant data structure in sync with its source. I could also rewrite my "mapping" data structure to be two parallel lists instead of one list of 2-ples, but this data structure is more readable and extensible and handles empty lists more naturally.

    13. rhettinger commented on Apr 14, 2010

      @rhettinger
      Contributor

      I'll take another look at this one.

    14. 35 remaining items

    15. NeilGirdhar commented on May 26, 2019

      NeilGirdharmannequin
      Mannequin

      The problem with key=lambda item: -item is that item cannot always be easily negated. For example, tuples are often used as keys.

    16. remilapeyre commented on May 26, 2019

      remilapeyremannequin
      Mannequin

      Can you not use functools.cmp_to_key() for this?

      Here's an example:

      >>> import bisect, functools
      >>> l = [('f', 5), ('d', 3), ('c', 2), ('b', 1), ('a', 0)]
      >>> def cmp(a, b):
      ...     if a > b: return -1
      ...     if a < b: return 1
      ...     return 0
      ... 
      >>> bisect.bisect(l, ('e', 4), key=functools.cmp_to_key(cmp))
      1
      >>> l
      [('f', 5), ('d', 3), ('c', 2), ('b', 1), ('a', 0)]
      >>> bisect.insort(l, ('e', 4), key=functools.cmp_to_key(cmp))
      >>> l
      [('f', 5), ('e', 4), ('d', 3), ('c', 2), ('b', 1), ('a', 0)]
    17. GPery commented on Jun 11, 2019

      GPerymannequin
      Mannequin

      I opened a relevant PR, #11781.

      I believe a key parameter is inferior to a comparison callback. The former is a specific case of the latter, and in my use case would force me to create another class to serve as a comparator for my objects, at which point I might as well wrap them and add __lt__.

      Furthermore, I believe this is more in-line with similar standard functions in other languages such as C++ (std::sort), Java (PriorityQueue) or Rust (slice.sort_by).

    18. mdickinson commented on Jun 11, 2019

      @mdickinson
      Member

      I opened a relevant PR, #11781.

      Did you mean #13970 ?

    19. GPery commented on Jun 11, 2019

      GPerymannequin
      Mannequin

      I did, thanks!

    20. added and removed on Aug 21, 2020
    21. rhettinger commented on Oct 20, 2020

      @rhettinger
      Contributor

      New changeset 871934d by Raymond Hettinger in branch 'master':
      bpo-4356: Add key function support to the bisect module (GH-20556)
      871934d

    22. pablogsal commented on Sep 14, 2021

      @pablogsal
      Member

      New changeset 1aaa859 by Pablo Galindo Salgado in branch 'main':
      bpo-4356: Mention the new key arguments for the bisect module APIs in the 3.10 What's new (GH-28339)
      1aaa859

    23. pablogsal commented on Sep 14, 2021

      @pablogsal
      Member

      New changeset dda5ff2 by Miss Islington (bot) in branch '3.10':
      bpo-4356: Mention the new key arguments for the bisect module APIs in the 3.10 What's new (GH-28339) (GH-28340)
      dda5ff2

    24. pablogsal commented on Oct 4, 2021

      @pablogsal
      Member

      New changeset 7128864 by Pablo Galindo (Miss Islington (bot)) in branch '3.10':
      bpo-4356: Mention the new key arguments for the bisect module APIs in the 3.10 What's new (GH-28339) (GH-28340)
      7128864

    25. transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    Labels

    3.10 (EOL)end of lifestdlibStandard Library Python modules in the Lib/ directorytype-featureA feature request or enhancement

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions