> On Jun 7, 2020, at 4:43 PM, Jean-Paul Calderone <exar...@twistedmatrix.com> > wrote: > > On Sun, Jun 7, 2020 at 7:22 PM Craig Rodrigues <rodr...@crodrigues.org > <mailto:rodr...@crodrigues.org>> wrote: > I have merged a few PR's to trunk which eliminate hundreds of errors > encountered with: > > tox -e mypy > > I think we can take several passes with more PR's to whack away all these > mypy errors, > and turn on mypy as part of the default CI for Twisted. > > I have seen a few errors like: > > src/twisted/words/protocols/jabber/sasl_mechanisms.py:47:1: error: > 'Anonymous' is missing following 'ISASLMechanism' interface members: > getResponse. [misc] > class Anonymous(object): > src/twisted/words/protocols/jabber/sasl_mechanisms.py:61:1: error: 'Plain' is > missing following 'ISASLMechanism' interface members: getResponse. [misc] > class Plain(object): > src/twisted/internet/_dumbwin32proc.py:110:1: error: 'Process' is missing > following 'twisted.internet.interfaces.ITransport' interface members: > getHost, getPeer. [misc] > class Process(_pollingfile._PollingTimer, BaseProcess): > src/twisted/internet/process.py:959:1: error: 'PTYProcess' is missing > following 'twisted.internet.interfaces.ITransport' interface members: > getHost, getPeer. [misc] > class PTYProcess(abstract.FileDescriptor, _BaseProcess): > src/twisted/internet/process.py:959:1: error: 'PTYProcess' is missing > following 'IProcessTransport' interface members: closeChildFD, writeToChild. > [misc] > class PTYProcess(abstract.FileDescriptor, _BaseProcess): > src/twisted/internet/base.py:504:1: error: 'ReactorBase' is missing following > 'IReactorCore' interface members: run. [misc] > class ReactorBase(PluggableResolverMixin) > > > For a class to properly implement a Zope interface, is it mandatory that it > implement every method in that interface? > > Yes. > > > If we modify the classes with mypy errors to properly implement these methods > (even with no-ops) is that the correct > way to go? > > Who does this serve? I would say no, this is not correct. If a type > declares it implements an interface and it cannot provide useful > implementations of every method/attribute, then it made a mistake in its > declaration or the interface has the wrong methods/attributes. >
First of all, it's totally awesome that we're catching these problems with mypy! Thank you to everyone who contributed to get this set up. I think that different errors probably indicate different problems. Without a lot of motivated consumers of this interface information, it's easy to slip up, and we have slipped up. And without accurate interface information it's hard to be a discerning consumer! So there's a bit of a chicken-and-egg problem here, and mypy will help us resolve that paradox, and let people really depend on these. To opine on the ones listed above specifically, in case this can be more readily generalized: Anonymous & Plain missing getResponse is probably something they can get away with due to the specifics of how SASL works with those specific mechanisms. They should still provide an implementation, although it would be fine if it was one that simply raised an explanatory exception explaining why they don't expect to get called in their specific cases. PTYProcess and Process's missing attributes are just bugs. I thought they were even already-filed bugs, but I was remembering this one: https://twistedmatrix.com/trac/ticket/4585. There was also https://twistedmatrix.com/trac/ticket/6606 . As I said: easy to screw this up without any way to consistently check; we've screwed it up in other ways in the past. We should implement all of these methods. ReactorBase is another depressing artifact of our inheritance-obsessed initial design, which I hold out hope that we may recover from before my 50th birthday. The idea here being that subclasses of ReactorBase should be implementing those interfaces, so it'll go ahead and declare them for you. Now, ReactorBase probably just shouldn't be public in the first place, but this specific attribute of its declaration is simply wrong; it doesn't implement those interfaces and it should not say that it does. -glyph
_______________________________________________ Twisted-Python mailing list Twisted-Python@twistedmatrix.com https://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
