Opened 5 years ago

Closed 5 years ago

#6198 defect closed fixed (fixed)

'in' operator support not implemented for twisted.python.modules.PythonPath

Reported by: Itamar Turner-Trauring Owned by: Jean-Paul Calderone
Priority: low Milestone:
Component: core Keywords:
Cc: Jordan Hagan Branch: branches/modules-contains-6198
branch-diff, diff-cov, branch-cov, buildbot
Author: exarkun

Description

>>> from twisted.python.modules import theSystemPath
>>> theSystemPath["os"]
PythonModule<'os'>
>>> "os" in theSystemPath
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "twisted/python/modules.py", line 699, in __getitem__
    if '.' in modname:
TypeError: argument of type 'int' is not iterable

Attachments (1)

pythonpath-contains-6198.patch (1.4 KB) - added by Jordan Hagan 5 years ago.

Download all attachments as: .zip

Change History (10)

Changed 5 years ago by Jordan Hagan

comment:1 Changed 5 years ago by Jordan Hagan

Cc: Jordan Hagan added
Keywords: review added

comment:2 Changed 5 years ago by Jean-Paul Calderone

Owner: set to Jean-Paul Calderone
Status: newassigned

comment:3 Changed 5 years ago by Jean-Paul Calderone

Author: exarkun
Branch: branches/modules-contains-6198

(In [36617]) Branching to 'modules-contains-6198'

comment:4 Changed 5 years ago by Jean-Paul Calderone

(In [36618]) apply pythonpath-contains-6198.patch

refs #6198

comment:5 Changed 5 years ago by Jean-Paul Calderone

Keywords: review removed

Thanks! This looks pretty good. I have a few comments though:

  1. A test should cover one case. Since unittest uses exceptions to signal failures, any assertions that follow a failing assertion won't run. This means failures may exist that won't get reported, if they happen to be checked after another failure. Testing each case in a dedicated test method provides better reporting by showing all failures instead of just some (which means you don't have to run the test suite as many times, and you can be more confident that once you fix some failures, all tests will be passing).
  2. Use L{} to refer to functions, classes, or modules in a docstring.
  3. Test method docstrings should not use the word should.
  4. This new feature should be described by a news fragment file.

These are pretty straightforward changes. I'll make them and, if buildbot is green, merge. Thanks.

comment:6 Changed 5 years ago by Jean-Paul Calderone

(In [36619]) address review comments

refs #6198

comment:7 Changed 5 years ago by Jean-Paul Calderone

comment:8 Changed 5 years ago by Jean-Paul Calderone

(In [36622]) minor doc improvement

refs #6198

comment:9 Changed 5 years ago by Jean-Paul Calderone

Resolution: fixed
Status: assignedclosed

(In [36623]) Merge modules-contains-6198

Author: ephess Reviewer: exarkun Fixes: #6198

Implement the in operator for right-hand values of type twisted.python.modules.PythonPath.

Note: See TracTickets for help on using tickets.