On Fri, Aug 25, 2017 at 9:49 AM, Thomas Kluyver <thomas@kluyver.me.uk> wrote:
Can I gently ask everyone involved to consider whether the notimplemented/error discussion is verging into bikeshedding (http://bikeshed.org/)?
The technical arguments I have seen so far are: - The exception can include a message - The return value can't 'bubble up' from the internals of a hook like an exception
I believe Nick also feels that an important advantage of NotImplementedError is: if a frontend doesn't bother to properly implement the spec and special case NotImplemented, then you'll end up eventually getting some obscure error like "'NotImplementedType' object has no attribute ..." when it tries to treat it as a normal return value. With NotImplementedError, if the frontend doesn't treat it specially, the default is to treat it like other exceptions and show a proper traceback and error message. So lazy frontends give better UX for NotImplementedError than NotImplemented. Personally, I don't find the argument about lazy frontends terribly compelling because if we're assuming that we're hitting some buggy corner case with code not following the spec, then we should also consider the case of accidentally bubbled NotImplementedErrors. Between these two cases, an accidentally bubbled NotImplementedError causes even more confusing outcomes (the build frontend may silently start invoking other things! vs. a suboptimal error message), and it's harder to guard against (both designs require properly written frontends to contain a few lines of code special casing NotImplemented/NotImplementedError; only NotImplementedError also requires all careful *backends* to contain just-in-case try/except guards). Another minor point that's made me less happy with NotImplemented is: originally I thought we could make it a general fact about all the hooks that returning "NotImplemented" should be treated the same as if the hook were undefined. (Which is pretty much how it works for __binop__ methods.) On further consideration though I don't think this is a good idea. (Rationale: it's not really what we want for get_build_requires_for_sdist, & if we define future hooks that normally have no return value then there's a danger of buggy frontends missing it entirely, & it wouldn't have worked for Nick's suggestion that build_wheel(build_directory=foo) triggering a NotImplemented should fall back to build_wheel(build_directory=None), which is gone from the spec now but suggests that this could cause problems in the future.) So the bottom line of all this is that if we do go with NotImplemented, I now think it should only be a defined return value for get_requires_for_build_sdist and build_sdist, and should have special "sorry I can't do that Dave" semantics that are different from e.g. a missing get_requires_for_build_sdist hook. All of which will work fine, it's just less... aesthetically pleasing. Personally, I still have a weak preference for NotImplemented over NotImplementedError, but I don't care enough to have a big fight about it. It sounds like Nick and Donald are the only two folks who really have strong opinions here: can the two of you work something out? Should we flip a coin? -n -- Nathaniel J. Smith -- https://vorpus.org