Opened 4 years ago

Closed 4 years ago

#6198 defect closed fixed (fixed)

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

Reported by: itamar Owned by: exarkun
Priority: low Milestone:
Component: core Keywords:
Cc: ephess Branch: branches/modules-contains-6198
branch-diff, diff-cov, branch-cov, buildbot
Author: exarkun


>>> from twisted.python.modules import theSystemPath
>>> theSystemPath["os"]
>>> "os" in theSystemPath
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "twisted/python/", 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 ephess 4 years ago.

Download all attachments as: .zip

Change History (10)

Changed 4 years ago by ephess

comment:1 Changed 4 years ago by ephess

  • Cc ephess added
  • Keywords review added

comment:2 Changed 4 years ago by exarkun

  • Owner set to exarkun
  • Status changed from new to assigned

comment:3 Changed 4 years ago by exarkun

  • Author set to exarkun
  • Branch set to branches/modules-contains-6198

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

comment:4 Changed 4 years ago by exarkun

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

refs #6198

comment:5 Changed 4 years ago by exarkun

  • 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 4 years ago by exarkun

(In [36619]) address review comments

refs #6198

comment:8 Changed 4 years ago by exarkun

(In [36622]) minor doc improvement

refs #6198

comment:9 Changed 4 years ago by exarkun

  • Resolution set to fixed
  • Status changed from assigned to closed

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