On Fri, Aug 25, 2017 at 2:17 PM, xoviat <xoviat@gmail.com> wrote:
Nathaniel:
What do you think of the proposal regarding DistutilsUnsupportedOperation?
So yeah, we could potentially say: 1) Every backend must have an attribute SdistBuildNotSupportedError, which is a type object subclassing Exception 2) When invoking the sdist build hooks, the frontend does something like: try: requires = backend.get_requires_for_build_sdist() except backend.SdistBuildNotSupportedError: switch_to_fallback_path() It's not ridiculous. If we did this then the name should be more specific than "DistutilsUnsupportedOperation", plus distutils has nothing to do with this. Also I don't think we'd ever want to move the exception into the stdlib, because the advantages are minor compared to the transition costs. And it does have the advantage that it resolves Nick's concern (IIUC) and also solves the accidental bubbling problem. I'm not sure if it would make Donald happy or not. It also provides a general template for how to handle custom errors in future hooks. The main annoyance would be that every backend has to contain some boilerplate like class SdistBuildNotSupportedError(Exception): pass even though most of them that won't ever raise this error, because otherwise the frontend will get an AttributeError in its except: statement. This is an awkward wart. To mitigate it, I guess we could make it optional for backends to export the exception, and frontends would instead write: try: requires = backend.get_requires_for_build_sdist() except getattr(backend, "SdistBuildNotSupportedError", ()): invoke_fallbacks() That's also kind of awkward, but... could be worse? -n
2017-08-25 16:13 GMT-05:00 Nathaniel Smith <njs@pobox.com>:
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 _______________________________________________ Distutils-SIG maillist - Distutils-SIG@python.org https://mail.python.org/mailman/listinfo/distutils-sig
-- Nathaniel J. Smith -- https://vorpus.org