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.
serhiy-storchaka
left a comment
serhiy-storchaka
left a comment
There was a problem hiding this comment.
Please add an entry in What's New.
| 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?
| 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.
| 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.
| 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. |
serhiy-storchaka
left a comment
serhiy-storchaka
left a comment
There was a problem hiding this comment.
LGTM, but @vadmium may suggest better wording.
4 participants
The
__getitem__methods of DOMEventStream, FileInputand FileWrapper classes ignore their 'index'
parameters and return the next item instead.
https://bugs.python.org/issue9372