bpo-9372: Deprecate several __getitem__ methods #8609
Conversation
The __getitem__ methods of DOMEventStream, FileInput and FileWrapper classes ignore their 'index' parameters and return the next item instead.
Doc/library/fileinput.rst
Outdated
| @@ -169,6 +169,9 @@ available for subclassing as well: | |||
| .. deprecated-removed:: 3.6 3.8 | |||
| The *bufsize* parameter. | |||
|
|
|||
| .. deprecated:: 3.8 | |||
| Support for :meth:`sequence protocol <__getitem__>` is deprecated. | |||
There was a problem hiding this comment.
Actually the sequence protocol never was supported. It requires __len__ and __reverse__, and better support of __getitem__.
There was a problem hiding this comment.
You're right. Do you have a suggestion for wording? Would something like "Support for __getitem__ method is deprecated." be suitable?
Lib/test/test_fileinput.py
Outdated
| r'Use iterator protocol instead'): | ||
| with FileInput(files=[t]) as fi: | ||
| with self.assertRaises(RuntimeError): | ||
| fi[1] |
There was a problem hiding this comment.
I suggest to use fi[0] which works now and returns "line1\n".
There was a problem hiding this comment.
I used fi[1] on purpose to show that __getitem__ doesn't work as expected (e.g. it would return 'line2\n' if it does)
There was a problem hiding this comment.
This is tested in other test (test__getitem__invalid_key). We need to test that even using a "valid" key is deprecated.
| @@ -138,6 +139,22 @@ def _ignore_deprecated_imports(ignore=True): | |||
| yield | |||
|
|
|||
|
|
|||
| def ignore_warnings(*, category): | |||
There was a problem hiding this comment.
Why it is a keyword-only parameter? For a single argument I expect a positional parameter.
There was a problem hiding this comment.
There are many warning-related APIs both in the stdlib and test.support and it's easy to forget how to use them, so I just tried to make the API more strict, easy to remember, and avoid misusing of the function.
Lib/fileinput.py
Outdated
| @@ -259,6 +259,13 @@ def __next__(self): | |||
| # repeat with next file | |||
|
|
|||
| def __getitem__(self, i): | |||
| import warnings | |||
| warnings.warn( | |||
| "FileInput's __getitem__ method is deprecated. " | |||
There was a problem hiding this comment.
The end user likely uses indexing instead of direct calling of __getitem__. What about "Indexing FileInput is deprecated."?
There was a problem hiding this comment.
How about "Indexing of a FileInput object is deprecated." or "Support for indexing a FileInput object is deprecated."
@vadmium do you have a suggestion on wording of the deprecation message?
|
I've addressed all of your review comments. |
The
__getitem__methods of DOMEventStream, FileInputand FileWrapper classes ignore their 'index'
parameters and return the next item instead.
https://bugs.python.org/issue9372