Skip to content

Deprecate using random.randrange() with non-integers #81500

Description

@serhiy-storchaka
BPO 37319
Nosy @tim-one, @rhettinger, @mdickinson, @serhiy-storchaka
PRs
  • bpo-37319: Deprecated support of non-integer arguments in random.randrange(). #19112
  • 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 = <Date 2021-01-25.21:03:21.912>
    created_at = <Date 2019-06-17.19:52:35.511>
    labels = ['type-feature', 'library', '3.10']
    title = 'Deprecate using random.randrange() with non-integers'
    updated_at = <Date 2021-01-25.21:03:21.911>
    user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2021-01-25.21:03:21.911>
    actor = 'serhiy.storchaka'
    assignee = 'none'
    closed = True
    closed_date = <Date 2021-01-25.21:03:21.912>
    closer = 'serhiy.storchaka'
    components = ['Library (Lib)']
    creation = <Date 2019-06-17.19:52:35.511>
    creator = 'serhiy.storchaka'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 37319
    keywords = ['patch']
    message_count = 15.0
    messages = ['345892', '345935', '345937', '346106', '364819', '364822', '364823', '364834', '364839', '364857', '364872', '364875', '380095', '383918', '385663']
    nosy_count = 4.0
    nosy_names = ['tim.peters', 'rhettinger', 'mark.dickinson', 'serhiy.storchaka']
    pr_nums = ['19112']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue37319'
    versions = ['Python 3.10']

    Activity

    1. serhiy-storchaka commented on Jun 17, 2019

      @serhiy-storchaka
      MemberAuthor

      Unlike other range()-like functions random.randrange() accepts not only integers and integer-like objects (implementing __index__), but any numbers with integral values, like 3.0 or Decimal(3). In 3.8 using __int__ for implicit converting to C integers were deprecated, and likely will be forbidden in 3.9. __index__ is now preferable.

      I propose to deprecate accepting non-integer arguments in random.randrange() and use __index__ instead of __int__ for converting values. At end it will lead to simpler and faster code. Instead of

              istart = _int(start)
              if istart != start:
                  raise ValueError("non-integer arg 1 for randrange()")

      we could use just

              start = _index(start)

      It will help also in converting the randrange() implementation to C.

      See also bpo-37315.

    2. added
      stdlibStandard Library Python modules in the Lib/ directory
      type-featureA feature request or enhancement
      on Jun 17, 2019
    3. rhettinger commented on Jun 17, 2019

      @rhettinger
      Contributor

      While there isn't anything intrinsically wrong with using index(), the premise of this issue is essentially a claim that most Python code that uses int() is wrong. That seems rather bold and flies in the face of common practices. I don't really like the trend of constantly rearranging code in awkward ways to solve problems that no one actually has.

      Also, I disagree with the notion that the downstream functions in the random module should be written in C. That makes them more bug prone and opaque (like when Victor broken seeding for 3.6.0). The trend for rewriting in C should be restricted to core building blocks like the underlying Mersenne Twister. Otherwise, we have to do dual maintenance for years and essentially advertise to people that we don't think our own language is good enough to use. Over time, I am less and less able to show people standard library code as it has become complexified with coding strategies well beyond the norms of day to day Python programmers. ISTM that most proposals to rewrite something in C come from folks who like rewriting things in C rather than from users or from people who do long term maintenance of the modules. Sidenote: we're now getting new core devs who don't read or write C, so each transformation like them excludes their ability to participate).

    4. rhettinger commented on Jun 17, 2019

      @rhettinger
      Contributor

      We probably should not do this until we're willing to give PEP-8 guidance recommending operator.index() as preferable to int() for all cases needing integer offsets. Maybe we do want to go down that path or maybe we just stick with "practicality beats purity" and not create any new learning issues. The int() function is popular and understandable, but it is rare to find a user who knows and cares about index(). ISTM that randrange(3.0) isn't a problem in real life.

    5. serhiy-storchaka commented on Jun 20, 2019

      @serhiy-storchaka
      MemberAuthor

      Why this code uses int() at all? My guess is that it was added for earlier detection of user errors, otherwise the user could get a TypeError or AttributeError with unrelated message, or even a wrong result. int() is used just for type checking. But it is not good tool for this. int() does several different actions:

      1. Parses string representation of integer.

      2. Converts a real number to an integer with some truncation.

      3. Losslessly converts an int-like number into an instance of int.

      The first two options are not applicable here, so we need an additional check

              if istart != start:
                  raise ValueError("non-integer arg 1 for randrange()")

      And this is not perfect: for randrange('5') we get a ValueError instead of expected TypeError.

      operator.index() is better tool for this.

    6. serhiy-storchaka commented on Mar 22, 2020

      @serhiy-storchaka
      MemberAuthor

      Many functions that accepts non-integer arguments (except float) and truncate them to integers emits a deprecation warning since 3.8 (see bpo-36048 and bpo-20092). They will be errors in 3.10 (see 37999).

    7. rhettinger commented on Mar 22, 2020

      @rhettinger
      Contributor

      I'm generally against "fixing" things that aren't broken. AFAICT, this has never been an issue for any real user over its 20 year history. Also, the current PR makes the code look gross and may impact code that is currently working. Tim, this is mostly your code, what do you think?

    8. serhiy-storchaka commented on Mar 22, 2020

      @serhiy-storchaka
      MemberAuthor

      Using index() simplifies the code. Adding deprecation warnings complicates it. The PR consists of two commits, look at the first one to see how the code can finally look.

      If you think that the deprecation is not needed, we can just keep the simplified version.

    9. tim-one commented on Mar 23, 2020

      @tim-one
      Member

      I have scant memory of working on this code. But back then Python wasn't at all keen to enforce type checking in harmless contexts. If, e.g., someone found it convenient to pass integers that happened to be in float format to randrange(), why not? Going "tsk, tsk, tsk" won't help them get their work done ;-)

      If we were restarting from scratch, I'd be happy enough to say "screw floats here - _and_ screw operator.index() - this function requires ints, period". But, as is, I see insufficient benefit to potentially making code that's been working fine for decades "deprecated".

      Yes, I realize the code may eventually become marginally faster then. That doesn't, to my eyes, outweigh the certainty of annoying some users.

    10. removed their assignment
      on Mar 23, 2020
    11. serhiy-storchaka commented on Mar 23, 2020

      @serhiy-storchaka
      MemberAuthor

      If you are against deprecating this "feature", it should be at least documented and covered by tests.

    12. rhettinger commented on Mar 23, 2020

      @rhettinger
      Contributor

      How about we just leave it alone. AFAICT nothing is broken -- no user is having any issues. We really don't need to go through every tool in the standard library that uses int() and document that it accepts any type that defines __int__, nor do we need to test those cases. I understand that you are bugged by this but it isn't a real problem.

    13. tim-one commented on Mar 23, 2020

      @tim-one
      Member

      It's neither "bug" nor "feature" - it's an inherited quirk CPython maintains to avoid annoying users for no good reason.

      If you absolutely have to "do something", how about adding a mere footnote? The docs already imply the args have the same restrictions as for range(). A footnote could add that, to preserve historical compatibility, CPython also accepts arguments x for which int(x) == x, and uses int(x) in such cases - but portable code shouldn't rely on that.

      But nobody cares, so putting that in the main part of the docs would also annoy 99.99% of users for no good reason ;-)

    14. rhettinger commented on Mar 23, 2020

      @rhettinger
      Contributor

      Marking this as rejected. Nothing good would come from obfuscating the docs to note a minor implementation detail.

    15. serhiy-storchaka commented on Oct 31, 2020

      @serhiy-storchaka
      MemberAuthor

      See bpo-42222.

      As a side effect this change provides 10% speed up (which can be lager after the end of the deprecation period).

    16. rhettinger commented on Dec 28, 2020

      @rhettinger
      Contributor

      I merged in PR 23064. If you want to make further modifications or improvements that would be welcome.

    17. serhiy-storchaka commented on Jan 25, 2021

      @serhiy-storchaka
      MemberAuthor

      New changeset f066bd9 by Serhiy Storchaka in branch 'master':
      bpo-37319: Improve documentation, code and tests of randrange. (GH-19112)
      f066bd9

    18. 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

    No one assigned

      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