#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: fs@… Branch: branches/modules-contains-6198
(diff, github, buildbot, log)
Author: exarkun Launchpad Bug:

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 ephess 20 months ago.

Download all attachments as: .zip

Change History (10)

Changed 20 months ago by ephess

comment:1 Changed 20 months ago by ephess

  • Cc fs@… added
  • Keywords review added

comment:2 Changed 19 months ago by exarkun

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

comment:3 Changed 19 months ago by exarkun

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

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

comment:4 Changed 19 months ago by exarkun

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

refs #6198

comment:5 Changed 19 months 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 19 months ago by exarkun

(In [36619]) address review comments

refs #6198

comment:8 Changed 19 months ago by exarkun

(In [36622]) minor doc improvement

refs #6198

comment:9 Changed 19 months 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.