req_rate is a namedtuple type rather than instance

[gvx]

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]

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.

[rhettinger]

• 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]

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:

• Please follow our commit style at https://devguide.python.org/committing/#commit-messages
• We need a NEWS entry. You can find details at https://devguide.python.org/committing/#what-s-new-and-news-entries (we don't need to add an entry in Doc/whatsnew/ so you can safely ignore it)

[rhettinger]

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]

What is a reason of making req_rate a named tuple?

[rhettinger]

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]

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]

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]

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

[rhettinger]

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