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

req_rate is a namedtuple type rather than instance #75506

Closed
gvx mannequin opened this issue Sep 1, 2017 · 10 comments
Closed

req_rate is a namedtuple type rather than instance #75506

gvx mannequin opened this issue Sep 1, 2017 · 10 comments
Assignees
Labels
3.7 (EOL) end of life stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@gvx
Copy link
Mannequin

gvx mannequin commented Sep 1, 2017

BPO 31325
Nosy @rhettinger, @berkerpeksag, @serhiy-storchaka, @gvx
PRs
  • bpo-31325: bug in RobotFileParser.parse #3259
  • bpo-31325: Fix usage of namedtuple in RobotFileParser.parse() #4529
  • [3.6] bpo-31325: Fix usage of namedtuple in RobotFileParser.parse() (GH-4529) #4533
  • 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/berkerpeksag'
    closed_at = <Date 2017-11-23.23:58:51.954>
    created_at = <Date 2017-09-01.15:15:22.782>
    labels = ['3.7', 'type-bug', 'library']
    title = 'req_rate is a namedtuple type rather than instance'
    updated_at = <Date 2017-11-23.23:58:51.954>
    user = 'https://github.com/gvx'

    bugs.python.org fields:

    activity = <Date 2017-11-23.23:58:51.954>
    actor = 'rhettinger'
    assignee = 'berker.peksag'
    closed = True
    closed_date = <Date 2017-11-23.23:58:51.954>
    closer = 'rhettinger'
    components = ['Library (Lib)']
    creation = <Date 2017-09-01.15:15:22.782>
    creator = 'gvx'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 31325
    keywords = ['patch']
    message_count = 10.0
    messages = ['301127', '301134', '301148', '301152', '303528', '303550', '303553', '303561', '306861', '306862']
    nosy_count = 4.0
    nosy_names = ['rhettinger', 'berker.peksag', 'serhiy.storchaka', 'gvx']
    pr_nums = ['3259', '4529', '4533']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue31325'
    versions = ['Python 3.6', 'Python 3.7']

    @gvx
    Copy link
    Mannequin Author

    gvx mannequin commented Sep 1, 2017

    Finally, urllib/robotparser.py appears to contain a bug in the
    following:

    req_rate = collections.namedtuple('req_rate',
    'requests seconds')
    entry.req_rate = req_rate
    entry.req_rate.requests = int(numbers[0])
    entry.req_rate.seconds = int(numbers[1])

    As I read it, this should fail as the req_rate is immutable.

    Ref: https://news.ycombinator.com/item?id=15136961

    They are mistaken in the nature of the bug, which is that req_rate is
    a namedtuple type, rather than an instance. As such, it is actually
    mutable, causing the assignments to not fail. Every entry creates a
    separate req_rate type, so it all works, but not in the way it should.

    @gvx gvx mannequin added 3.7 (EOL) end of life stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Sep 1, 2017
    @rhettinger
    Copy link
    Contributor

    • The named tuple class should begin with a capital letter and be fully self-documenting: "RequestRate".

    • The creation of the named tuple class should be done only once, not on every call. Instead only a new instance should be creating on every call:

         entry.req_rate = req_rate(RequestRate)
    • There needs to be a test.

    • The docstring should be updated to include the name of the class refer to the term named tuple instead of the namedtuple() factory function:

      • Returns the contents of the Request-rate parameter from
      • robots.txt in the form of a :func:~collections.namedtuple
      • (requests, seconds). If there is no such parameter or it doesn't
      • Returns the contents of the Request-rate parameter from
      • robots.txt as a :term:named tuple RequestRate(requests, seconds).
      • If there is no such parameter or it doesn't

    @berkerpeksag
    Copy link
    Member

    Good catch and thank you for turning the bug report in the HN thread to a pull request! I agree with all of Raymond's comments.

    I have two more comments:

    @rhettinger
    Copy link
    Contributor

    There was a typo in my previous message. The instantiation code should be:

       entry.req_rate = RequestRate(int(numbers[0]), int(numbers[1]))

    @serhiy-storchaka
    Copy link
    Member

    What is a reason of making req_rate a named tuple?

    @rhettinger
    Copy link
    Contributor

    What is a reason of making req_rate a named tuple?

    I don't know the original reason but it seems like a normal use of named tuples to make the data more self-describing to help with debugging and also to support field access by name, rr.requests or rr.seconds is clearer than rr[0] or rr[1].

    @serhiy-storchaka
    Copy link
    Member

    This is a normal use of named tuples for adding access by name to tuple results. But req_rate never was a tuple. Nobody used rr[0].

    @rhettinger
    Copy link
    Contributor

    There is no rule that something had to be a tuple at some point in its history before becoming a named tuple. This use seems perfectly reasonable to me.

    @rhettinger
    Copy link
    Contributor

    New changeset 3df02db by Raymond Hettinger (Berker Peksag) in branch 'master':
    bpo-31325: Fix usage of namedtuple in RobotFileParser.parse() (bpo-4529)
    3df02db

    @rhettinger
    Copy link
    Contributor

    New changeset ff847d1 by Raymond Hettinger (Miss Islington (bot)) in branch '3.6':
    bpo-31325: Fix usage of namedtuple in RobotFileParser.parse() (GH-4529) (bpo-4533)
    ff847d1

    @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
    3.7 (EOL) end of life stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants