Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(199)

#25958: Implicit ABCs have no means of "anti-registration"

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 9 months ago by abarnert
Modified:
1 year, 2 months ago
Reviewers:
guido, storchaka, vadmium+py
CC:
gvanrossum, rhettinger, Nick Coghlan, stoneleaf, devnull_psf.upfronthosting.co.za, Martin Panter, storchaka, abarnert, levkivskyi, Mariatta
Visibility:
Public.

Patch Set 1 #

Total comments: 18

Patch Set 2 #

Total comments: 1

Patch Set 3 #

Total comments: 20

Patch Set 4 #

Total comments: 18

Patch Set 5 #

Patch Set 6 #

Patch Set 7 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Doc/library/collections.abc.rst View 1 2 3 4 5 6 1 chunk +2 lines, -1 line 0 comments Download
Doc/reference/datamodel.rst View 1 2 3 4 5 6 3 chunks +18 lines, -1 line 0 comments Download
Lib/_collections_abc.py View 1 2 3 4 5 6 14 chunks +29 lines, -49 lines 0 comments Download
Lib/test/test_augassign.py View 1 2 3 4 5 6 2 chunks +8 lines, -0 lines 0 comments Download
Lib/test/test_binop.py View 1 2 3 4 5 6 2 chunks +49 lines, -1 line 0 comments Download
Lib/test/test_bool.py View 1 2 3 4 5 6 1 chunk +11 lines, -0 lines 0 comments Download
Lib/test/test_bytes.py View 1 2 3 4 5 6 1 chunk +30 lines, -0 lines 0 comments Download
Lib/test/test_collections.py View 1 2 3 4 5 6 10 chunks +53 lines, -7 lines 0 comments Download
Lib/test/test_contains.py View 1 2 3 4 5 6 1 chunk +25 lines, -0 lines 0 comments Download
Lib/test/test_enumerate.py View 1 2 3 4 5 6 2 chunks +8 lines, -1 line 0 comments Download
Lib/test/test_iter.py View 1 2 3 4 5 6 2 chunks +12 lines, -0 lines 0 comments Download
Lib/test/test_unicode.py View 1 2 3 4 5 6 2 chunks +23 lines, -0 lines 0 comments Download
Objects/enumobject.c View 1 2 3 4 5 6 2 chunks +10 lines, -2 lines 0 comments Download
Objects/typeobject.c View 1 2 3 4 5 6 2 chunks +14 lines, -0 lines 0 comments Download

Messages

Total messages: 11
gvanrossum
When I manually trigger the code in typeobject.c:5827, I get a segfault; I'm surprised no ...
1 year, 9 months ago #1
storchaka_gmail.com
http://bugs.python.org/review/25958/diff/16288/Doc/reference/datamodel.rst File Doc/reference/datamodel.rst (right): http://bugs.python.org/review/25958/diff/16288/Doc/reference/datamodel.rst#newcode1067 Doc/reference/datamodel.rst:1067: Setting a special method to ``None`` indicates that the ...
1 year, 9 months ago #2
gvanrossum
I don't see why Reversible shouldn't inherit from Iterable. After all, how would you define ...
1 year, 9 months ago #3
abarnert
I think Serhiy is right that making __reversed__ and __contains__ error messages consistent with __iter__ ...
1 year, 9 months ago #4
storchaka_gmail.com
http://bugs.python.org/review/25958/diff/16288/Lib/test/test_collections.py File Lib/test/test_collections.py (right): http://bugs.python.org/review/25958/diff/16288/Lib/test/test_collections.py#newcode692 Lib/test/test_collections.py:692: class J: On 2016/01/05 23:28:55, abarnert wrote: > On ...
1 year, 9 months ago #5
storchaka_gmail.com
http://bugs.python.org/review/25958/diff/16297/Lib/test/test_binop.py File Lib/test/test_binop.py (right): http://bugs.python.org/review/25958/diff/16297/Lib/test/test_binop.py#newcode391 Lib/test/test_binop.py:391: class E(object): (object) is not needed in Python 3. ...
1 year, 9 months ago #6
abarnert
http://bugs.python.org/review/25958/diff/16297/Lib/test/test_binop.py File Lib/test/test_binop.py (right): http://bugs.python.org/review/25958/diff/16297/Lib/test/test_binop.py#newcode391 Lib/test/test_binop.py:391: class E(object): On 2016/01/06 21:55:31, storchaka wrote: > (object) ...
1 year, 9 months ago #7
abarnert
http://bugs.python.org/review/25958/diff/16297/Lib/test/test_bytes.py File Lib/test/test_bytes.py (right): http://bugs.python.org/review/25958/diff/16297/Lib/test/test_bytes.py#newcode839 Lib/test/test_bytes.py:839: class BytesSubclassBlocked(BytesSubclass): On 2016/01/06 21:55:31, storchaka wrote: > Is ...
1 year, 9 months ago #8
Martin Panter
https://bugs.python.org/review/25958/diff/16297/Lib/test/test_binop.py File Lib/test/test_binop.py (right): https://bugs.python.org/review/25958/diff/16297/Lib/test/test_binop.py#newcode391 Lib/test/test_binop.py:391: class E(object): On 2016/01/06 23:04:32, abarnert wrote: > On ...
1 year, 9 months ago #9
abarnert
http://bugs.python.org/review/25958/diff/16301/Lib/test/test_augassign.py File Lib/test/test_augassign.py (right): http://bugs.python.org/review/25958/diff/16301/Lib/test/test_augassign.py#newcode87 Lib/test/test_augassign.py:87: "Blocks inheritance, and fallback to __add__" On 2016/01/09 02:34:57, ...
1 year, 9 months ago #10
Martin Panter
1 year, 8 months ago #11
http://bugs.python.org/review/25958/diff/16301/Lib/test/test_binop.py
File Lib/test/test_binop.py (right):

http://bugs.python.org/review/25958/diff/16301/Lib/test/test_binop.py#newcode421
Lib/test/test_binop.py:421: e, f, s, x = E(), F(), S(), X()
On 2016/01/18 20:28:42, abarnert wrote:
> On 2016/01/09 02:34:57, vadmium wrote:
> > Maybe move the classes into the test method, to avoid filling up globals
with
> > single-letter class names?
> 
> Some of the classes are used in multiple test methods, so that would require
> defining them twice.

Yes the common base class E is shared; that’s why I suggested a longer name for
it.

> More generally, what is the problem you're trying to avoid here? I'm following
a
> pattern set by not only this module, but dozens of other stdlib test modules,
> and I'm not aware of that pattern having caused any problems.

Yes, and I think I am guilty of adding the existing V class to this file. My
concern is that as we add more single-letter global classes, it gets easier for
someone to accidentally mask one of them with their own class, and silently
break a test. Anyway, it’s not (yet) a big deal.

http://bugs.python.org/review/25958/diff/16301/Lib/test/test_collections.py
File Lib/test/test_collections.py (right):

http://bugs.python.org/review/25958/diff/16301/Lib/test/test_collections.py#n...
Lib/test/test_collections.py:711: Counter().values(), (lambda: (yield))(),
On 2016/01/18 20:28:42, abarnert wrote:
> On 2016/01/09 02:34:57, vadmium wrote:
> > but I would find this way less cryptic:
> > 
> > def generator():
> >     yield
> 
> It's also in `test_Iterator`, `test_Generator`, `test_Sized`,
`test_Container`,
> and `test_Callable`.
> 
> And I don't understand the actual problem here. Do you really think people
> reading this test will not be able to figure out that we're testing that
> generators are iterable, iterator, generators, not sized, not containers, not
> callable, and not reversible? And do you think two extra lines of code in
every
> one of those functions will make it easier, rather than harder, to see how
well
> the tests cover the library?

Well I did figure it out, but it initially threw me, and I doubt I am the only
one. I had to think to figure out it does what you want. Instead of two lines in
every method, there could be a shared global generator function.

http://bugs.python.org/review/25958/diff/16301/Lib/test/test_unicode.py
File Lib/test/test_unicode.py (right):

http://bugs.python.org/review/25958/diff/16301/Lib/test/test_unicode.py#newco...
Lib/test/test_unicode.py:1214: self.assertRaises(TypeError, "{}".format, m)
On 2016/01/18 20:28:42, abarnert wrote:
> On 2016/01/09 02:34:57, vadmium wrote:
> > Also directly test format(m)
> 
> Why? We don't do that for any of the other cases. Presumably if `str.format`
> doesn't actually call the same code as `format`, that's caught somewhere else.
> (Or, if not, surely it should be tested with a much simpler case than this
one.)

I suspect you are right that this test method is specifically testing the
str.format() method. It looks like there is code in test_builtins which does
general tests for the plain format() function.
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7