Nathaniel seems to be busy with other things at the moment, so I hope he won't mind me passing on this list of things he'd like to resolve with the draft PEP. I'll quote his comments and put my responses inline.
- needs some sort of NotImplemented(Error) handling specified
We've gone round return-value vs exception a few times, but I don't think that debate is particularly heated, so it probably just needs someone to say "this is what we're doing". I can be that someone if needs be. But does anyone feel strongly about it?
- The out-of-tree build example (that makes an sdist and then builds it) seems like exactly the kind of implementation that Donald says he doesn't want to see? OTOH I think Nick said he wants to see a more elaborated specification of out-of-tree build strategies with this specifically as one of them. - Nick has suggested that the to-be-defined NotImplemented(Error) handling can be used by build_wheel to indicate that it can't do out-of-tree builds, so maybe the frontend should try an in-tree build instead. I don't know how to convert this into a real spec, though -- like in general, if I'm a frontend and I call `hook(*args, **kwargs)` and it says "can't do that", then how do I know what the problem is and what I should try instead? - What happens if prepare_build_metadata returns NotImplemented / raises NotImplementedError? - I don't understand how out-of-tree builds and prepare_build_metadata are supposed to interact. - Also, AFAICT the out-of-tree build feature has no motivation anymore. The only mentioned use case is to support incremental build features in automatic build pipelines like Travis, but there are a number of these build pipelines, and lots of existing build systems that support out-of-tree builds, and AFAICT no-one actually uses them together like this. (I think it's because most build systems' out-of-tree build features are designed on the assumption that there's a human running the show who will deal with any edge cases.)
I believe the out-of-tree build option Nathaniel is referring to is the build_directory parameter to build_wheel(). His proposed APIs remove that parameter, and elsewhere in his email he describes that no-one seems to want it: everyone thinks someone else wants it. By my understanding, the reasons for including the build_directory parameter are: 1. Without an explicit build directory, the developers of pip are concerned that build backends will modify the source tree and cause issues which users report as bugs to pip. 2. A frontend-controlled build directory could be used for caching intermediate build artifacts. This was a secondary argument after we had the idea, and we've never really fleshed out how we expect it to work. There's also a concern that if the first round of frontends always use an empty tempdir, backends will end up relying on that. I think this second argument is a weak one unless we spend the time to figure out the details. Do other people see the situation in a similar way? How might we move forwards on this?
- needs some sort of conclusion to the srcdir-on-sys.path issue.
While Nathaniel is in the minority regarding srcdir-on-sys.path, he argues that most of us are assuming a default position without really thinking through it, which is certainly true of me. I don't feel strongly about this topic, but I do want to come to a conclusion. As Nathaniel does feel strongly about it, does anyone object to either: A. Leaving the source dir off sys.path when loading the hooks, and making a special backend which exposes hooks from the CWD. B. Adding another key in the TOML file to control whether hooks can be loaded from the source dir. I can live with either, but I prefer A, because it means fewer options to describe in the spec. Thomas
I mean is this golang or Python? In Python, you raise notimplementederror. On Aug 24, 2017 8:17 AM, "Thomas Kluyver" <thomas@kluyver.me.uk> wrote:
Nathaniel seems to be busy with other things at the moment, so I hope he won't mind me passing on this list of things he'd like to resolve with the draft PEP. I'll quote his comments and put my responses inline.
- needs some sort of NotImplemented(Error) handling specified
We've gone round return-value vs exception a few times, but I don't think that debate is particularly heated, so it probably just needs someone to say "this is what we're doing". I can be that someone if needs be. But does anyone feel strongly about it?
- The out-of-tree build example (that makes an sdist and then builds it) seems like exactly the kind of implementation that Donald says he doesn't want to see? OTOH I think Nick said he wants to see a more elaborated specification of out-of-tree build strategies with this specifically as one of them. - Nick has suggested that the to-be-defined NotImplemented(Error) handling can be used by build_wheel to indicate that it can't do out-of-tree builds, so maybe the frontend should try an in-tree build instead. I don't know how to convert this into a real spec, though -- like in general, if I'm a frontend and I call `hook(*args, **kwargs)` and it says "can't do that", then how do I know what the problem is and what I should try instead? - What happens if prepare_build_metadata returns NotImplemented / raises NotImplementedError? - I don't understand how out-of-tree builds and prepare_build_metadata are supposed to interact. - Also, AFAICT the out-of-tree build feature has no motivation anymore. The only mentioned use case is to support incremental build features in automatic build pipelines like Travis, but there are a number of these build pipelines, and lots of existing build systems that support out-of-tree builds, and AFAICT no-one actually uses them together like this. (I think it's because most build systems' out-of-tree build features are designed on the assumption that there's a human running the show who will deal with any edge cases.)
I believe the out-of-tree build option Nathaniel is referring to is the build_directory parameter to build_wheel(). His proposed APIs remove that parameter, and elsewhere in his email he describes that no-one seems to want it: everyone thinks someone else wants it.
By my understanding, the reasons for including the build_directory parameter are:
1. Without an explicit build directory, the developers of pip are concerned that build backends will modify the source tree and cause issues which users report as bugs to pip. 2. A frontend-controlled build directory could be used for caching intermediate build artifacts. This was a secondary argument after we had the idea, and we've never really fleshed out how we expect it to work. There's also a concern that if the first round of frontends always use an empty tempdir, backends will end up relying on that. I think this second argument is a weak one unless we spend the time to figure out the details.
Do other people see the situation in a similar way? How might we move forwards on this?
- needs some sort of conclusion to the srcdir-on-sys.path issue.
While Nathaniel is in the minority regarding srcdir-on-sys.path, he argues that most of us are assuming a default position without really thinking through it, which is certainly true of me. I don't feel strongly about this topic, but I do want to come to a conclusion. As Nathaniel does feel strongly about it, does anyone object to either:
A. Leaving the source dir off sys.path when loading the hooks, and making a special backend which exposes hooks from the CWD. B. Adding another key in the TOML file to control whether hooks can be loaded from the source dir.
I can live with either, but I prefer A, because it means fewer options to describe in the spec.
Thomas _______________________________________________ Distutils-SIG maillist - Distutils-SIG@python.org https://mail.python.org/mailman/listinfo/distutils-sig
On Thu, Aug 24, 2017, at 02:21 PM, xoviat wrote:
I mean is this golang or Python? In Python, you raise notimplementederror. But there's a NotImplemented singleton in Python as well. The argument for using a return value is that the hook code has to deliberately return that, whereas a NotImplementedError could bubble up from some internal call, in which case it should really be registered as an error.
On 24 August 2017 at 10:28, Thomas Kluyver <thomas@kluyver.me.uk> wrote:
On Thu, Aug 24, 2017, at 02:21 PM, xoviat wrote:
I mean is this golang or Python? In Python, you raise notimplementederror.
But there's a NotImplemented singleton in Python as well. The argument for using a return value is that the hook code has to deliberately return that, whereas a NotImplementedError could bubble up from some internal call, in which case it should really be registered as an error.
This! And a backend that wanted a bubbled up NotImplementedError (or some other exception) to mean it actually doesn't support an operation, can catch the error and return. The ideal Pythonic way would be to raise a specific exception (something like PEP517UnsupportedOperation), but since it's not a builtin we would be left with the problem of defining it somewhere, which would become a dependency of all backends, and no, we don't want that. All this has been hashed and rehashed before. I think we can move on from this argument and define `return NotImplemented` as the way to define a backend operation as unsupported.
On 24 August 2017 at 23:11, Thomas Kluyver <thomas@kluyver.me.uk> wrote:
Nathaniel seems to be busy with other things at the moment, so I hope he won't mind me passing on this list of things he'd like to resolve with the draft PEP. I'll quote his comments and put my responses inline.
- needs some sort of NotImplemented(Error) handling specified
We've gone round return-value vs exception a few times, but I don't think that debate is particularly heated, so it probably just needs someone to say "this is what we're doing". I can be that someone if needs be. But does anyone feel strongly about it?
Aye, I do, and it should be "raise NotImplementedError('Explanation of the failed check')" Rationale: - Python isn't C or Go, so we indicate failures with exceptions, not error codes (NotImplemented is an necessary performance hack for operand coercion in tight loops, not an example to be emulated in other APIs) - allows the backend to provide information on what went wrong - means an unhandled backend error results in a traceback pointing to where the build failed, not some later point in the frontend code - if a backend developer is sufficiently worried about accidentally propagating NotImplementedError that they want to pretend they're not writing Python any more, they can use this idiom: def public_hook_api(*args, **kwds): try: result, error_msg = _internal_hook_implementation(*args, **kwds) except NotImplementedError as exc: raise RuntimeError("Unexpected NotImplementedError") from exc if result is NotImplemented: raise NotImplementedError(error_msg) return result That provides the backend with all the same assurances against accidentally letting NotImplementedError escape that a return code based public API would, without frontends even needing to be aware of the backend developer's aversion to reporting errors as exceptions.
- The out-of-tree build example (that makes an sdist and then builds it) seems like exactly the kind of implementation that Donald says he doesn't want to see? OTOH I think Nick said he wants to see a more elaborated specification of out-of-tree build strategies with this specifically as one of them.
Not really - I raised that prospect because Nathaniel was insisting that out-of-tree builds would be too hard for backend developers to implement, and I pointed out that they really weren't that complicated: - if you're wrapping a backend that supports them, pass down the directory setting - if you're not, implement them as semantically equivalent to build sdist -> unpack sdist -> build wheel - since build_sdist can fail with NotImplementedError, out-of-tree builds are also permitted to fail that way
- Nick has suggested that the to-be-defined NotImplemented(Error) handling can be used by build_wheel to indicate that it can't do out-of-tree builds, so maybe the frontend should try an in-tree build instead. I don't know how to convert this into a real spec, though -- like in general, if I'm a frontend and I call `hook(*args, **kwargs)` and it says "can't do that", then how do I know what the problem is and what I should try instead?
The fallback chains are defined by frontends, not the spec, and they depend on: - what the frontend is trying to do - what the fronted is trying to prevent For the case of "build a wheel from a source tree" for example, a reasonable fallback chain might be: - try build_sdist - if that raises NotImplementedError, try an explicitly out-of-tree build_wheel call - if that also raises NotImplementedError, try an unqualified build_wheel call It would also be equally reasonable to skip either or both of the first two options.
- What happens if prepare_build_metadata returns NotImplemented / raises NotImplementedError?
Up to the frontend, but failing outright seems reasonable (since there isn't any real reason to expect generating the entire wheel to succeed if generating the metadata failed)
- I don't understand how out-of-tree builds and prepare_build_metadata are supposed to interact.
They don't, since the backend should only implement prepare_build_metadata if it can generate the metadata without actually triggering a full build of all the binary artifacts.
- Also, AFAICT the out-of-tree build feature has no motivation anymore. The only mentioned use case is to support incremental build features in automatic build pipelines like Travis, but there are a number of these build pipelines, and lots of existing build systems that support out-of-tree builds, and AFAICT no-one actually uses them together like this. (I think it's because most build systems' out-of-tree build features are designed on the assumption that there's a human running the show who will deal with any edge cases.)
I believe the out-of-tree build option Nathaniel is referring to is the build_directory parameter to build_wheel(). His proposed APIs remove that parameter, and elsewhere in his email he describes that no-one seems to want it: everyone thinks someone else wants it.
By my understanding, the reasons for including the build_directory parameter are:
1. Without an explicit build directory, the developers of pip are concerned that build backends will modify the source tree and cause issues which users report as bugs to pip.
This is the motivation that Donald and Paul have indicated isn't necessarily valid any more, since they'd be OK with a setup where pip uses the following build model by default: 1. First try the explicit build_sdist -> unpack sdist -> build_wheel path 2. If build_sdist raises NotImplementedError, fall back to trying build_wheel directly
2. A frontend-controlled build directory could be used for caching intermediate build artifacts. This was a secondary argument after we had the idea, and we've never really fleshed out how we expect it to work. There's also a concern that if the first round of frontends always use an empty tempdir, backends will end up relying on that. I think this second argument is a weak one unless we spend the time to figure out the details.
Do other people see the situation in a similar way? How might we move forwards on this?
As long as Donald & Paul are OK with it for pip, I'm OK with omitting the build_directory parameter for now - since we're going to make supporting it optional regardless, that means it doesn't matter as much whether its in the initial iteration of the API or not.
- needs some sort of conclusion to the srcdir-on-sys.path issue.
While Nathaniel is in the minority regarding srcdir-on-sys.path, he argues that most of us are assuming a default position without really thinking through it, which is certainly true of me. I don't feel strongly about this topic, but I do want to come to a conclusion. As Nathaniel does feel strongly about it, does anyone object to either:
A. Leaving the source dir off sys.path when loading the hooks, and making a special backend which exposes hooks from the CWD. B. Adding another key in the TOML file to control whether hooks can be loaded from the source dir.
I'm mainly interested in the way things fail, and who has the power to fix them when they break. - when the default is "source tree is set as sys.path[0]": - that's essentially the same as the status quo with setup.py - if a project's build process has a name shadowing problem, the publisher is going to hit that locally and fix it prior to release - when the default is "the source tree is not set as sys.path[0]", we know that: - any setuptools backend is going to have to ensure that the old default is in place when running setup.py - any backend that runs a Python script is going to end up with the CWD as sys.path[0] in that script anyway due to Python's default behaviour - if a project self-hosts its own build backend, we either have to say "that's not supported", or else provide a way to change the default So I think this is a case where we have an established precedent (i.e. the way setup.py currently works), and the potential for accidental name shadowing doesn't offer a sufficiently compelling rationale for changing that default. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On Thu, Aug 24, 2017 at 7:52 AM, Nick Coghlan <ncoghlan@gmail.com> wrote:
- I don't understand how out-of-tree builds and prepare_build_metadata are supposed to interact.
They don't, since the backend should only implement prepare_build_metadata if it can generate the metadata without actually triggering a full build of all the binary artifacts.
The reason I had this on the list is that in many build systems, these two steps are intimately linked. E.g. in autoconf, "figure out which dependencies I'm going to use" (aka prepare_build_metadata) and "set up an out-of-tree build directory" are the same operation. Alternatively "sniff the system and config files etc. to figure out what what dependencies we're going to use" could be just another build target -- but then it still requires that the build directory be set up first. -n -- Nathaniel J. Smith -- https://vorpus.org
Nathaniel: Are you okay with what has been proposed? On Aug 24, 2017 7:29 PM, "Nathaniel Smith" <njs@pobox.com> wrote:
On Thu, Aug 24, 2017 at 7:52 AM, Nick Coghlan <ncoghlan@gmail.com> wrote:
- I don't understand how out-of-tree builds and prepare_build_metadata are supposed to interact.
They don't, since the backend should only implement prepare_build_metadata if it can generate the metadata without actually triggering a full build of all the binary artifacts.
The reason I had this on the list is that in many build systems, these two steps are intimately linked. E.g. in autoconf, "figure out which dependencies I'm going to use" (aka prepare_build_metadata) and "set up an out-of-tree build directory" are the same operation. Alternatively "sniff the system and config files etc. to figure out what what dependencies we're going to use" could be just another build target -- but then it still requires that the build directory be set up first.
-n
-- Nathaniel J. Smith -- https://vorpus.org _______________________________________________ Distutils-SIG maillist - Distutils-SIG@python.org https://mail.python.org/mailman/listinfo/distutils-sig
On Aug 24, 2017, at 10:52 AM, Nick Coghlan <ncoghlan@gmail.com> wrote:
Aye, I do, and it should be "raise NotImplementedError('Explanation of the failed check')"
Rationale:
- Python isn't C or Go, so we indicate failures with exceptions, not error codes (NotImplemented is an necessary performance hack for operand coercion in tight loops, not an example to be emulated in other APIs) - allows the backend to provide information on what went wrong - means an unhandled backend error results in a traceback pointing to where the build failed, not some later point in the frontend code - if a backend developer is sufficiently worried about accidentally propagating NotImplementedError that they want to pretend they're not writing Python any more, they can use this idiom:
def public_hook_api(*args, **kwds): try: result, error_msg = _internal_hook_implementation(*args, **kwds) except NotImplementedError as exc: raise RuntimeError("Unexpected NotImplementedError") from exc if result is NotImplemented: raise NotImplementedError(error_msg) return result
That provides the backend with all the same assurances against accidentally letting NotImplementedError escape that a return code based public API would, without frontends even needing to be aware of the backend developer's aversion to reporting errors as exceptions.
I’m not really a fan of using NotImplementedError instead of NotImplemented. We’re not going to implement it by showing a traceback to where the NotImplementedError happened because it’s not an error case. And really that’s the important bit here, this is not an error case (as far as the API is concerned), this is just one of the possible return values that this function can produce. A front end may *choose* to make this an error case of course, but that is at a different layer than this API is operating. It’s arguably not even the correct usage of NotImplementedError, since that is (and I quote from the Python docs): "In user defined base classes, abstract methods should raise this exception when they require derived classes to override the method, or while the class is being developed to indicate that the real implementation still needs to be added.” This is not a case of some real implementation not having yet been added or some stub code getting called before it’s ready. This use case more closely resembles NotImplemented. — Donald Stufft
According to the documentation, NotImplemented isn't appropriate either, as is for binary operations only. There is no one value that's taylor made for this situation, but an exception may be more appropriate as the underlying cause is probably an error. On Aug 25, 2017 11:11 AM, "Donald Stufft" <donald@stufft.io> wrote:
On Aug 24, 2017, at 10:52 AM, Nick Coghlan <ncoghlan@gmail.com> wrote:
Aye, I do, and it should be "raise NotImplementedError('Explanation of the failed check')"
Rationale:
- Python isn't C or Go, so we indicate failures with exceptions, not error codes (NotImplemented is an necessary performance hack for operand coercion in tight loops, not an example to be emulated in other APIs) - allows the backend to provide information on what went wrong - means an unhandled backend error results in a traceback pointing to where the build failed, not some later point in the frontend code - if a backend developer is sufficiently worried about accidentally propagating NotImplementedError that they want to pretend they're not writing Python any more, they can use this idiom:
def public_hook_api(*args, **kwds): try: result, error_msg = _internal_hook_implementation(*args, **kwds) except NotImplementedError as exc: raise RuntimeError("Unexpected NotImplementedError") from exc if result is NotImplemented: raise NotImplementedError(error_msg) return result
That provides the backend with all the same assurances against accidentally letting NotImplementedError escape that a return code based public API would, without frontends even needing to be aware of the backend developer's aversion to reporting errors as exceptions.
I’m not really a fan of using NotImplementedError instead of NotImplemented. We’re not going to implement it by showing a traceback to where the NotImplementedError happened because it’s not an error case. And really that’s the important bit here, this is not an error case (as far as the API is concerned), this is just one of the possible return values that this function can produce.
A front end may *choose* to make this an error case of course, but that is at a different layer than this API is operating.
It’s arguably not even the correct usage of NotImplementedError, since that is (and I quote from the Python docs): "In user defined base classes, abstract methods should raise this exception when they require derived classes to override the method, or while the class is being developed to indicate that the real implementation still needs to be added.”
This is not a case of some real implementation not having yet been added or some stub code getting called before it’s ready. This use case more closely resembles NotImplemented.
— Donald Stufft
_______________________________________________ Distutils-SIG maillist - Distutils-SIG@python.org https://mail.python.org/mailman/listinfo/distutils-sig
On Aug 25, 2017, at 12:38 PM, xoviat <xoviat@gmail.com> wrote:
According to the documentation, NotImplemented isn't appropriate either, as is for binary operations only. There is no one value that's taylor made for this situation, but an exception may be more appropriate as the underlying cause is probably an error.
The underlying cause is *not* an error just the same as it’s not an error for a __eq__ to not know how to test equality against a specific `other`. The underlying cause is explicitly “I do not want to or know how to handle this case” not “I’ve accidentally called some code that wasn’t implemented yet”. The use case is almost exactly the same as the binop use case. — Donald Stufft
By the way, what is the status on sys.path? On Aug 25, 2017 11:45 AM, "Donald Stufft" <donald@stufft.io> wrote:
On Aug 25, 2017, at 12:38 PM, xoviat <xoviat@gmail.com> wrote:
According to the documentation, NotImplemented isn't appropriate either, as is for binary operations only. There is no one value that's taylor made for this situation, but an exception may be more appropriate as the underlying cause is probably an error.
The underlying cause is *not* an error just the same as it’s not an error for a __eq__ to not know how to test equality against a specific `other`. The underlying cause is explicitly “I do not want to or know how to handle this case” not “I’ve accidentally called some code that wasn’t implemented yet”. The use case is almost exactly the same as the binop use case.
— Donald Stufft
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 don't think the discussion of semantics is going to go anywhere: they are both reasonable ways for the backend to reply "sorry, Dave, I can't do that". On Fri, Aug 25, 2017, at 05:38 PM, xoviat wrote:
According to the documentation, NotImplemented isn't appropriate either, as is for binary operations only. There is no one value that's taylor made for this situation, but an exception may be more appropriate as the underlying cause is probably an error.> On Aug 25, 2017 11:11 AM, "Donald Stufft" <donald@stufft.io> wrote:
On Aug 24, 2017, at 10:52 AM, Nick Coghlan <ncoghlan@gmail.com> wrote:>>> Aye, I do, and it should be "raise NotImplementedError('Explanation of>>> the failed check')"
Rationale:
- Python isn't C or Go, so we indicate failures with exceptions, not>>> error codes (NotImplemented is an necessary performance hack for operand coercion in tight loops, not an example to be emulated in other APIs) - allows the backend to provide information on what went wrong - means an unhandled backend error results in a traceback pointing to>>> where the build failed, not some later point in the frontend code - if a backend developer is sufficiently worried about accidentally>>> propagating NotImplementedError that they want to pretend they're not>>> writing Python any more, they can use this idiom:
def public_hook_api(*args, **kwds): try: result, error_msg = _internal_hook_implementation(*args, **kwds)>>> except NotImplementedError as exc: raise RuntimeError("Unexpected NotImplementedError") from exc>>> if result is NotImplemented: raise NotImplementedError(error_msg) return result
That provides the backend with all the same assurances against accidentally letting NotImplementedError escape that a return code based public API would, without frontends even needing to be aware of>>> the backend developer's aversion to reporting errors as exceptions.>>
I’m not really a fan of using NotImplementedError instead of NotImplemented. We’re not going to implement it by showing a traceback to where the NotImplementedError happened because it’s not an error case. And really that’s the important bit here, this is not an error case (as far as the API is concerned), this is just one of the possible return values that this function can produce.>> A front end may *choose* to make this an error case of course, but that is at a different layer than this API is operating.>> It’s arguably not even the correct usage of NotImplementedError, since that is (and I quote from the Python docs): "In user defined base classes, abstract methods should raise this exception when they require derived classes to override the method, or while the class is being developed to indicate that the real implementation still needs to be added.”>> This is not a case of some real implementation not having yet been added or some stub code getting called before it’s ready. This use case more closely resembles NotImplemented.>> — Donald Stufft
_______________________________________________ Distutils-SIG maillist - Distutils-SIG@python.org https://mail.python.org/mailman/listinfo/distutils-sig
_________________________________________________ Distutils-SIG maillist - Distutils-SIG@python.org https://mail.python.org/mailman/listinfo/distutils-sig
We could support both options on the frontend side. It's only a tiny bit of duplication in pip. On Aug 25, 2017 11:50 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 don't think the discussion of semantics is going to go anywhere: they are both reasonable ways for the backend to reply "sorry, Dave, I can't do that".
On Fri, Aug 25, 2017, at 05:38 PM, xoviat wrote:
According to the documentation, NotImplemented isn't appropriate either, as is for binary operations only. There is no one value that's taylor made for this situation, but an exception may be more appropriate as the underlying cause is probably an error.
On Aug 25, 2017 11:11 AM, "Donald Stufft" <donald@stufft.io> wrote:
On Aug 24, 2017, at 10:52 AM, Nick Coghlan <ncoghlan@gmail.com> wrote:
Aye, I do, and it should be "raise NotImplementedError('Explanation of the failed check')"
Rationale:
- Python isn't C or Go, so we indicate failures with exceptions, not error codes (NotImplemented is an necessary performance hack for operand coercion in tight loops, not an example to be emulated in other APIs) - allows the backend to provide information on what went wrong - means an unhandled backend error results in a traceback pointing to where the build failed, not some later point in the frontend code - if a backend developer is sufficiently worried about accidentally propagating NotImplementedError that they want to pretend they're not writing Python any more, they can use this idiom:
def public_hook_api(*args, **kwds): try: result, error_msg = _internal_hook_implementation(*args, **kwds) except NotImplementedError as exc: raise RuntimeError("Unexpected NotImplementedError") from exc if result is NotImplemented: raise NotImplementedError(error_msg) return result
That provides the backend with all the same assurances against accidentally letting NotImplementedError escape that a return code based public API would, without frontends even needing to be aware of the backend developer's aversion to reporting errors as exceptions.
I’m not really a fan of using NotImplementedError instead of NotImplemented. We’re not going to implement it by showing a traceback to where the NotImplementedError happened because it’s not an error case. And really that’s the important bit here, this is not an error case (as far as the API is concerned), this is just one of the possible return values that this function can produce.
A front end may *choose* to make this an error case of course, but that is at a different layer than this API is operating.
It’s arguably not even the correct usage of NotImplementedError, since that is (and I quote from the Python docs): "In user defined base classes, abstract methods should raise this exception when they require derived classes to override the method, or while the class is being developed to indicate that the real implementation still needs to be added.”
This is not a case of some real implementation not having yet been added or some stub code getting called before it’s ready. This use case more closely resembles NotImplemented.
— Donald Stufft
_______________________________________________ Distutils-SIG maillist - Distutils-SIG@python.org https://mail.python.org/mailman/listinfo/distutils-sig
*_______________________________________________* Distutils-SIG maillist - Distutils-SIG@python.org https://mail.python.org/mailman/listinfo/distutils-sig
_______________________________________________ Distutils-SIG maillist - Distutils-SIG@python.org https://mail.python.org/mailman/listinfo/distutils-sig
On Aug 25, 2017, at 12:49 PM, 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/ <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 don't think the discussion of semantics is going to go anywhere: they are both reasonable ways for the backend to reply "sorry, Dave, I can't do that".
I don’t think they are both reasonable ways any more than it’s reasonable to do ``raise Return(value)`` instead of ``return value``. The semantics here are important because using exceptions for non-exceptional, non erroneous cases has *always*, in my experience, lead to weirdness [1]. [1] Like for example, StopIteration which was deemed so bad as to need to break backwards compatibility and break consistency with all other uses of exceptions just to handle the weirdness in a saner way. Unfortunately we can’t modify the Python interpreter to fix our weirdness that is going to happen. — Donald Stufft
Is pip going to fall back to building a wheel directly if any other error is raised? That's what happens with setup.py install. If so, then it may be fine if unexpected exceptions bubble up. On Aug 25, 2017 11:57 AM, "Donald Stufft" <donald@stufft.io> wrote:
On Aug 25, 2017, at 12:49 PM, 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 don't think the discussion of semantics is going to go anywhere: they are both reasonable ways for the backend to reply "sorry, Dave, I can't do that".
I don’t think they are both reasonable ways any more than it’s reasonable to do ``raise Return(value)`` instead of ``return value``. The semantics here are important because using exceptions for non-exceptional, non erroneous cases has *always*, in my experience, lead to weirdness [1].
[1] Like for example, StopIteration which was deemed so bad as to need to break backwards compatibility and break consistency with all other uses of exceptions just to handle the weirdness in a saner way. Unfortunately we can’t modify the Python interpreter to fix our weirdness that is going to happen.
— Donald Stufft
_______________________________________________ Distutils-SIG maillist - Distutils-SIG@python.org https://mail.python.org/mailman/listinfo/distutils-sig
On Aug 25, 2017, at 1:06 PM, xoviat <xoviat@gmail.com> wrote:
Is pip going to fall back to building a wheel directly if any other error is raised? That's what happens with setup.py install. If so, then it may be fine if unexpected exceptions bubble up.
I doubt it. It will likely just print out the error and fail. The current “assume any error means fallback” situation is not very good, because most errors are not recoverable and what happens is we just make things slower by trying to repeat the operation which is going to fail instead of bailing as son as we know it’s going to fail. — Donald Stufft
On 25 August 2017 at 18:06, xoviat <xoviat@gmail.com> wrote:
Is pip going to fall back to building a wheel directly if any other error is raised? That's what happens with setup.py install. If so, then it may be fine if unexpected exceptions bubble up.
Given that hooks need to be called in a subprocess (see https://www.python.org/dev/peps/pep-0517/#id15, "Frontends should call each hook in a fresh subprocess, so that backends are free to change process global state") there's no "bubbling up" involved at all. The frontend code would be something along the lines of hook_stub = ''' import backend try: backend.build_sdist(...) except NotImplementedError: sys.exit(1) sys.exit(0) ''' # Or... hook_stub = ''' import backend if backend.build_sdist(...) == NotImplemented: sys.exit(1) sys.exit(0) ''' if subprocess.call([sys.executable, "-c", hook_stub]) != 0: # We didn't build a sdist There's little or no opportunity here for letting exceptions bubble up to the user, or passing complex data back to the frontend. Ultimately, it's pretty much immaterial which form of reporting is used. Paul
The issue is about exceptions bubbling up to the interface level, not about crashing pip. For example, if some class raises notimplemented error and pip interprets that to mean it should call build_wheel. On Aug 25, 2017 1:23 PM, "Paul Moore" <p.f.moore@gmail.com> wrote:
On 25 August 2017 at 18:06, xoviat <xoviat@gmail.com> wrote:
Is pip going to fall back to building a wheel directly if any other error is raised? That's what happens with setup.py install. If so, then it may be fine if unexpected exceptions bubble up.
Given that hooks need to be called in a subprocess (see https://www.python.org/dev/peps/pep-0517/#id15, "Frontends should call each hook in a fresh subprocess, so that backends are free to change process global state") there's no "bubbling up" involved at all. The frontend code would be something along the lines of
hook_stub = ''' import backend try: backend.build_sdist(...) except NotImplementedError: sys.exit(1) sys.exit(0) ''' # Or... hook_stub = ''' import backend if backend.build_sdist(...) == NotImplemented: sys.exit(1) sys.exit(0) '''
if subprocess.call([sys.executable, "-c", hook_stub]) != 0: # We didn't build a sdist
There's little or no opportunity here for letting exceptions bubble up to the user, or passing complex data back to the frontend. Ultimately, it's pretty much immaterial which form of reporting is used.
Paul
The thing being bubbled up is a backend accidentally calling an API that has yet to be implemented (an error that should be reported) being bubbled up and erroneously handled as the backend reporting it doesn't support making a sdist (not an error, son no traceback). Sent from my iPhone
On Aug 25, 2017, at 2:23 PM, Paul Moore <p.f.moore@gmail.com> wrote:
There's little or no opportunity here for letting exceptions bubble up to the user, or passing complex data back to the frontend. Ultimately, it's pretty much immaterial which form of reporting is used.
I agree with Nick that exceptions are the way to do things in Python and with Donald that in general, Python's use of exceptions has caused problems. But I don't think that this forum is the correct place to discuss Python conventions, and so I would ordinarily say that we should just accept that there will be problems and use the NotImplementedError, assuming that "solution" doesn't cause a firestorm that delays acceptance for a significant period of time. The reasoning behind this is that Python has in general adopted this approach (Nick is right that they would have used NotImplementedError for binary operations except for performance issues) even with its problems. However, another solution was dismissed without much thought: using the "UnsupportedOperation" exception. We could for now simply require an extra two lines in all backends: class DistutilsUnsupportedOperation(RuntimeError): pass and then put UnsupportedOperation in the "distutils" library in 3.7, with an eventual move to that. It's not pretty but there have been plenty of hacks for backwards compatibility so far (it's rare to see a project without some sort of compat.py file). 2017-08-25 13:34 GMT-05:00 Donald Stufft <donald@stufft.io>:
The thing being bubbled up is a backend accidentally calling an API that has yet to be implemented (an error that should be reported) being bubbled up and erroneously handled as the backend reporting it doesn't support making a sdist (not an error, son no traceback).
Sent from my iPhone
On Aug 25, 2017, at 2:23 PM, Paul Moore <p.f.moore@gmail.com> wrote:
There's little or no opportunity here for letting exceptions bubble up to the user, or passing complex data back to the frontend. Ultimately, it's pretty much immaterial which form of reporting is used.
It's silly to require every backend to write code to guard against possible issues when we can structure the API to prevent those issues. Sent from my iPhone
On Aug 25, 2017, at 3:04 PM, xoviat <xoviat@gmail.com> wrote:
I agree with Nick that exceptions are the way to do things in Python and with Donald that in general, Python's use of exceptions has caused problems. But I don't think that this forum is the correct place to discuss Python conventions, and so I would ordinarily say that we should just accept that there will be problems and use the NotImplementedError, assuming that "solution" doesn't cause a firestorm that delays acceptance for a significant period of time. The reasoning behind this is that Python has in general adopted this approach (Nick is right that they would have used NotImplementedError for binary operations except for performance issues) even with its problems.
However, another solution was dismissed without much thought: using the "UnsupportedOperation" exception. We could for now simply require an extra two lines in all backends:
class DistutilsUnsupportedOperation(RuntimeError): pass
and then put UnsupportedOperation in the "distutils" library in 3.7, with an eventual move to that. It's not pretty but there have been plenty of hacks for backwards compatibility so far (it's rare to see a project without some sort of compat.py file).
2017-08-25 13:34 GMT-05:00 Donald Stufft <donald@stufft.io>:
The thing being bubbled up is a backend accidentally calling an API that has yet to be implemented (an error that should be reported) being bubbled up and erroneously handled as the backend reporting it doesn't support making a sdist (not an error, son no traceback).
Sent from my iPhone
On Aug 25, 2017, at 2:23 PM, Paul Moore <p.f.moore@gmail.com> wrote:
There's little or no opportunity here for letting exceptions bubble up to the user, or passing complex data back to the frontend. Ultimately, it's pretty much immaterial which form of reporting is used.
Do you mean in addition to the two lines proposed here? I envision the typical use case as attempting to do something that is required to build an sdist deep inside the backend, discovering that it cannot be done, and then raising the proposed exception. That's (other than the two lines for the exception, which can put in Python later) shorter than catching some custom exception in the hook and then returning NotImplemented as a result. If you don't use exceptions, you'll probably need to unwind the stack manually with return values. 2017-08-25 14:25 GMT-05:00 Donald Stufft <donald@stufft.io>:
It's silly to require every backend to write code to guard against possible issues when we can structure the API to prevent those issues.
Sent from my iPhone
On Aug 25, 2017, at 3:04 PM, xoviat <xoviat@gmail.com> wrote:
I agree with Nick that exceptions are the way to do things in Python and with Donald that in general, Python's use of exceptions has caused problems. But I don't think that this forum is the correct place to discuss Python conventions, and so I would ordinarily say that we should just accept that there will be problems and use the NotImplementedError, assuming that "solution" doesn't cause a firestorm that delays acceptance for a significant period of time. The reasoning behind this is that Python has in general adopted this approach (Nick is right that they would have used NotImplementedError for binary operations except for performance issues) even with its problems.
However, another solution was dismissed without much thought: using the "UnsupportedOperation" exception. We could for now simply require an extra two lines in all backends:
class DistutilsUnsupportedOperation(RuntimeError): pass
and then put UnsupportedOperation in the "distutils" library in 3.7, with an eventual move to that. It's not pretty but there have been plenty of hacks for backwards compatibility so far (it's rare to see a project without some sort of compat.py file).
2017-08-25 13:34 GMT-05:00 Donald Stufft <donald@stufft.io>:
The thing being bubbled up is a backend accidentally calling an API that has yet to be implemented (an error that should be reported) being bubbled up and erroneously handled as the backend reporting it doesn't support making a sdist (not an error, son no traceback).
Sent from my iPhone
On Aug 25, 2017, at 2:23 PM, Paul Moore <p.f.moore@gmail.com> wrote:
There's little or no opportunity here for letting exceptions bubble up to the user, or passing complex data back to the frontend. Ultimately, it's pretty much immaterial which form of reporting is used.
What if hooks returned instances of the exception class? That way it doesn't bubble up from internal code, it can carry a message, and we can all agree that it's a semantically terrible idea that doesn't fit with any conventions. Even I'm not sure if this is a serious suggestion. ;-) On Fri, Aug 25, 2017, at 08:31 PM, xoviat wrote:
Do you mean in addition to the two lines proposed here? I envision the typical use case as attempting to do something that is required to build an sdist deep inside the backend, discovering that it cannot be done, and then raising the proposed exception. That's (other than the two lines for the exception, which can put in Python later) shorter than catching some custom exception in the hook and then returning NotImplemented as a result. If you don't use exceptions, you'll probably need to unwind the stack manually with return values.> 2017-08-25 14:25 GMT-05:00 Donald Stufft <donald@stufft.io>:
It's silly to require every backend to write code to guard against possible issues when we can structure the API to prevent those issues.>> Sent from my iPhone
On Aug 25, 2017, at 3:04 PM, xoviat <xoviat@gmail.com> wrote:
I agree with Nick that exceptions are the way to do things in Python and with Donald that in general, Python's use of exceptions has caused problems. But I don't think that this forum is the correct place to discuss Python conventions, and so I would ordinarily say that we should just accept that there will be problems and use the NotImplementedError, assuming that "solution" doesn't cause a firestorm that delays acceptance for a significant period of time. The reasoning behind this is that Python has in general adopted this approach (Nick is right that they would have used NotImplementedError for binary operations except for performance issues) even with its problems.>>> However, another solution was dismissed without much thought: using the "UnsupportedOperation" exception. We could for now simply require an extra two lines in all backends:>>> class DistutilsUnsupportedOperation(RuntimeError): pass
and then put UnsupportedOperation in the "distutils" library in 3.7, with an eventual move to that. It's not pretty but there have been plenty of hacks for backwards compatibility so far (it's rare to see a project without some sort of compat.py file).>>> 2017-08-25 13:34 GMT-05:00 Donald Stufft <donald@stufft.io>:
The thing being bubbled up is a backend accidentally calling an API that has yet to be implemented (an error that should be reported) being bubbled up and erroneously handled as the backend reporting it doesn't support making a sdist (not an error, son no traceback).>>>> Sent from my iPhone
On Aug 25, 2017, at 2:23 PM, Paul Moore <p.f.moore@gmail.com> wrote:>>>> > There's little or no opportunity here for letting exceptions bubble up>>>> > to the user, or passing complex data back to the frontend. Ultimately,>>>> > it's pretty much immaterial which form of reporting is used.
Distutils-SIG maillist - Distutils-SIG@python.org https://mail.python.org/mailman/listinfo/distutils-sig
There's no need to return an instance of the exception class. As long as it is defined in the same name space as the hooks the front end will be able to access it. On Aug 25, 2017 3:15 PM, "Thomas Kluyver" <thomas@kluyver.me.uk> wrote:
What if hooks returned instances of the exception class? That way it doesn't bubble up from internal code, it can carry a message, and we can all agree that it's a semantically terrible idea that doesn't fit with any conventions.
Even I'm not sure if this is a serious suggestion. ;-)
On Fri, Aug 25, 2017, at 08:31 PM, xoviat wrote:
Do you mean in addition to the two lines proposed here? I envision the typical use case as attempting to do something that is required to build an sdist deep inside the backend, discovering that it cannot be done, and then raising the proposed exception. That's (other than the two lines for the exception, which can put in Python later) shorter than catching some custom exception in the hook and then returning NotImplemented as a result. If you don't use exceptions, you'll probably need to unwind the stack manually with return values.
2017-08-25 14:25 GMT-05:00 Donald Stufft <donald@stufft.io>:
It's silly to require every backend to write code to guard against possible issues when we can structure the API to prevent those issues.
Sent from my iPhone
On Aug 25, 2017, at 3:04 PM, xoviat <xoviat@gmail.com> wrote:
I agree with Nick that exceptions are the way to do things in Python and with Donald that in general, Python's use of exceptions has caused problems. But I don't think that this forum is the correct place to discuss Python conventions, and so I would ordinarily say that we should just accept that there will be problems and use the NotImplementedError, assuming that "solution" doesn't cause a firestorm that delays acceptance for a significant period of time. The reasoning behind this is that Python has in general adopted this approach (Nick is right that they would have used NotImplementedError for binary operations except for performance issues) even with its problems.
However, another solution was dismissed without much thought: using the "UnsupportedOperation" exception. We could for now simply require an extra two lines in all backends:
class DistutilsUnsupportedOperation(RuntimeError): pass
and then put UnsupportedOperation in the "distutils" library in 3.7, with an eventual move to that. It's not pretty but there have been plenty of hacks for backwards compatibility so far (it's rare to see a project without some sort of compat.py file).
2017-08-25 13:34 GMT-05:00 Donald Stufft <donald@stufft.io>:
The thing being bubbled up is a backend accidentally calling an API that has yet to be implemented (an error that should be reported) being bubbled up and erroneously handled as the backend reporting it doesn't support making a sdist (not an error, son no traceback).
Sent from my iPhone
On Aug 25, 2017, at 2:23 PM, Paul Moore <p.f.moore@gmail.com> wrote:
There's little or no opportunity here for letting exceptions bubble up to the user, or passing complex data back to the frontend. Ultimately, it's pretty much immaterial which form of reporting is used.
*_______________________________________________* Distutils-SIG maillist - Distutils-SIG@python.org https://mail.python.org/mailman/listinfo/distutils-sig
_______________________________________________ Distutils-SIG maillist - Distutils-SIG@python.org https://mail.python.org/mailman/listinfo/distutils-sig
On Fri, Aug 25, 2017, at 09:41 PM, xoviat wrote:
There's no need to return an instance of the exception class. As long as it is defined in the same name space as the hooks the front end will be able to access it. No, I meant NotImplementedError. As in:
if i_cant_do_it: return NotImplementedError(msg) Thereby neatly annoying all sides of the debate at once.
Genius! 2017-08-25 15:47 GMT-05:00 Thomas Kluyver <thomas@kluyver.me.uk>:
On Fri, Aug 25, 2017, at 09:41 PM, xoviat wrote:
There's no need to return an instance of the exception class. As long as it is defined in the same name space as the hooks the front end will be able to access it.
No, I meant NotImplementedError. As in:
if i_cant_do_it: return NotImplementedError(msg)
Thereby neatly annoying all sides of the debate at once.
On Fri, Aug 25, 2017 at 1:51 PM, Thomas Kluyver <thomas@kluyver.me.uk> wrote:
On Fri, Aug 25, 2017, at 09:50 PM, xoviat wrote:
Genius!
1% inspiration, 99% frustration :-P
This joke is so clever that I fear we may be forced to implement the solution after all, just to punish Thomas. -n -- Nathaniel J. Smith -- https://vorpus.org
While we're waiting for others to respond, what are we going to do with respect to the sys.path issue? I don't think there has been discussion on that yet. 2017-08-25 15:54 GMT-05:00 Nathaniel Smith <njs@pobox.com>:
On Fri, Aug 25, 2017 at 1:51 PM, Thomas Kluyver <thomas@kluyver.me.uk> wrote:
On Fri, Aug 25, 2017, at 09:50 PM, xoviat wrote:
Genius!
1% inspiration, 99% frustration :-P
This joke is so clever that I fear we may be forced to implement the solution after all, just to punish Thomas.
-n
-- Nathaniel J. Smith -- https://vorpus.org
On 25 August 2017 at 18:02, xoviat <xoviat@gmail.com> wrote:
While we're waiting for others to respond, what are we going to do with respect to the sys.path issue? I don't think there has been discussion on that yet.
There was plenty of discussion. The summary is: - removing srcdir from sys.path requires explicitly removing "" from the head of sys.path, which is not normally done. - it would break many setuptools based projects, probably forcing the setuptools backend to reinsert it, defeating the purpose of removing it in the first place. - if any backend launches a python based subprocess (on top of the one already launched by the frontend for calling the backend), "" goes right back to the head of sys.path, defeating the purpose of removing it in the first place - it will make it impossible for packages to carry or be their own backends, which is an important usecase for the backend packages themselves and for complex setups conspicuously absent from the discussion was an actual decision on whether to remove or just document the presence of srcdir on sys.path. Cheers, Leo
While we're waiting for others to respond, what are we going to do with respect to the sys.path issue? I don't think there has been discussion on that yet. I'm more or less persuaded by Nathaniel's argument that the source
- removing srcdir from sys.path requires explicitly removing "" from the head of sys.path, which is not normally done. Not necessarily. If the hook process is launched with a script at the command line, it is the script directory, not the cwd, that is added to sys.path. My WIP hook-calling module works this way:https://github.com/takluyver/pep517/blob/ee43a9334c377d7c37badcc8527cb7a8500... With this code, we would actually need to explicitly *add* the source
On Fri, Aug 25, 2017, at 10:02 PM, xoviat wrote: directory shouldn't be on sys.path, because: - Most projects won't need it to be - There are plausible ways it could go wrong - I think we could make a backend which loads the real hooks from the source dir, so there's a backdoor for projects that do want it. IIRC, however, Nick was still strongly in favour of including the source directory on sys.path, and he can usually persuade me of his position with a few emails. I do think that behaviour is more like the 'obvious thing' that Python programmers would expect. Leo: directory (CWD) to sys.path before importing the hook.
- it would break many setuptools based projects, probably forcing the setuptools backend to reinsert it, defeating the purpose of removing it in the first place. I don't think we should decide this based on what setuptools does; I expect it to need quite a bit of special handling in any case. Thomas
I'm more or less persuaded by Nathaniel's argument that the source directory shouldn't be on sys.path
I do too. There should be an option in pyproject.toml to disable this behavior though so that numpy can build itself. 2017-08-25 16:21 GMT-05:00 Thomas Kluyver <thomas@kluyver.me.uk>:
On Fri, Aug 25, 2017, at 10:02 PM, xoviat wrote:
While we're waiting for others to respond, what are we going to do with respect to the sys.path issue? I don't think there has been discussion on that yet.
I'm more or less persuaded by Nathaniel's argument that the source directory shouldn't be on sys.path, because:
- Most projects won't need it to be - There are plausible ways it could go wrong - I think we could make a backend which loads the real hooks from the source dir, so there's a backdoor for projects that do want it.
IIRC, however, Nick was still strongly in favour of including the source directory on sys.path, and he can usually persuade me of his position with a few emails. I do think that behaviour is more like the 'obvious thing' that Python programmers would expect.
Leo:
- removing srcdir from sys.path requires explicitly removing "" from the head of sys.path, which is not normally done.
Not necessarily. If the hook process is launched with a script at the command line, it is the script directory, not the cwd, that is added to sys.path. My WIP hook-calling module works this way: https://github.com/takluyver/pep517/blob/ee43a9334c377d7c37badcc8527cb7 a8500180f7/pep517/wrappers.py#L75
With this code, we would actually need to explicitly *add* the source directory (CWD) to sys.path before importing the hook.
- it would break many setuptools based projects, probably forcing the setuptools backend to reinsert it, defeating the purpose of removing it in the first place.
I don't think we should decide this based on what setuptools does; I expect it to need quite a bit of special handling in any case.
Thomas
On Fri, Aug 25, 2017, at 10:26 PM, xoviat wrote: > > I'm more or less persuaded by Nathaniel's argument that the source > > directory shouldn't be on sys.path> > I do too. There should be an option in pyproject.toml to disable this > behavior though so that numpy can build itself. I'm not sure that's necessary: - As I've described, a special shim backend could load hooks from the source directory. This doesn't need us to specify another option in the toml file.- Nathaniel is one of the core numpy developers, so if he's not asking for that switch, numpy presumably doesn't need it. :-)
On Fri, Aug 25, 2017 at 2:26 PM, xoviat <xoviat@gmail.com> wrote:
I'm more or less persuaded by Nathaniel's argument that the source directory shouldn't be on sys.path
I do too. There should be an option in pyproject.toml to disable this behavior though so that numpy can build itself.
My original proposal was to leave the srcdir off of sys.path, and then have a key in pyproject.toml like: [build-system] backend-python-path = ["."] Frontends would then do something like: os.chdir(srcdir) # This line is new to handle the above proposal: sys.path[:0] = [abspath(p) for p in config["build-system"].get("backend-python-path", [])] backend = resolve_entrypoint(config["build-system"]["build-backend"]) I don't have a strong opinion on whether we put this into PEP 517 (it's pretty light weight and doesn't interact with any other features AFAICT), or make it a followup PEP, or start out by deferring this option to a dedicated build backend, like: [build-system] requires = ["override_backend_path"] build-backend = "override_backend_path" [tool.override_backend_path] python-path = ["."] real-backend = "my_awesome:backend" These are all pretty similar. I think the big question for debate is: should sys.path be configurable, or not configurable? IIUC, the main argument for putting the source directory on the path was that extra configuration options are annoying, so we don't want one of those, but we do want to support in-tree backends (even though we expect that most projects won't use this), so we had better put the srcdir on sys.path. My feeling is that in practice, though, that the "no configuration, srcdir always on sys.path" approach is not going to hold up. So first, obviously, the hack above works just fine to make it configurable even if we don't put it in any PEP, so.... in fact it's configurable no matter what. Plus, sooner or later, someone's going to say "hey distutils-sig, I have a build backend package that I want to be able to bootstrap itself, AND I want to put my package inside src/ for reasons [1], wouldn't it be nice if I could put src/ on sys.path without jumping through hoops?". Or someone will release a new version of their build backend that adds a new dependency, or one of their transitive dependencies will release a new version that adds a new dependency, and it collides with some already-released package that uses that build-backend, and the project suffering the collision gets annoyed at being told they need to rearrange their source tree (retroactively, in already released versions!). And they'll come here and say "hey distutils-sig, can solve this problem once and for all?". And we'll be like... uh, fixing this would take what, <5 lines of new code in pip? Kinda hard to argue with that. So... it'll be configurable, one way or another. And if it's configurable... then the question is about whether the default configuration: is it srcdir on sys.path ("opt-out"), or srcdir not on sys.path ("opt-in"). And it seems to me that in this case, all the standard criteria say it should be opt-in. If it's opt-in, then everyone using build backends distributed on PyPI -- which we expect to be the vast majority of project -- never have to think about it and it does the right thing, with no risk of collisions or anything. In fact the only people who have to think about it are the ones implementing in-tree backends, and if you're already like, writing a whole backend and configuring your pyproject.toml to invoke it, then asking you to add one more line of configuration is really not a big deal. OTOH if it's opt-out, then it becomes Yet Another Bad Packaging Default, where conscientious package authors will fret about the risk of collisions and write blog posts about how every project needs to make sure to opt-out of this as a Best Practice, and I am so, so tired of those. -n [1] https://blog.ionelmc.ro/2014/05/25/python-packaging/#the-structure -- Nathaniel J. Smith -- https://vorpus.org
For the record, I agree with the proposals made in Nathaniel's last two emails. 2017-08-25 17:36 GMT-05:00 Nathaniel Smith <njs@pobox.com>:
On Fri, Aug 25, 2017 at 2:26 PM, xoviat <xoviat@gmail.com> wrote:
I'm more or less persuaded by Nathaniel's argument that the source directory shouldn't be on sys.path
I do too. There should be an option in pyproject.toml to disable this behavior though so that numpy can build itself.
My original proposal was to leave the srcdir off of sys.path, and then have a key in pyproject.toml like:
[build-system] backend-python-path = ["."]
Frontends would then do something like:
os.chdir(srcdir) # This line is new to handle the above proposal: sys.path[:0] = [abspath(p) for p in config["build-system"].get("backend-python-path", [])] backend = resolve_entrypoint(config["build-system"]["build-backend"])
I don't have a strong opinion on whether we put this into PEP 517 (it's pretty light weight and doesn't interact with any other features AFAICT), or make it a followup PEP, or start out by deferring this option to a dedicated build backend, like:
[build-system] requires = ["override_backend_path"] build-backend = "override_backend_path"
[tool.override_backend_path] python-path = ["."] real-backend = "my_awesome:backend"
These are all pretty similar.
I think the big question for debate is: should sys.path be configurable, or not configurable? IIUC, the main argument for putting the source directory on the path was that extra configuration options are annoying, so we don't want one of those, but we do want to support in-tree backends (even though we expect that most projects won't use this), so we had better put the srcdir on sys.path.
My feeling is that in practice, though, that the "no configuration, srcdir always on sys.path" approach is not going to hold up. So first, obviously, the hack above works just fine to make it configurable even if we don't put it in any PEP, so.... in fact it's configurable no matter what. Plus, sooner or later, someone's going to say "hey distutils-sig, I have a build backend package that I want to be able to bootstrap itself, AND I want to put my package inside src/ for reasons [1], wouldn't it be nice if I could put src/ on sys.path without jumping through hoops?". Or someone will release a new version of their build backend that adds a new dependency, or one of their transitive dependencies will release a new version that adds a new dependency, and it collides with some already-released package that uses that build-backend, and the project suffering the collision gets annoyed at being told they need to rearrange their source tree (retroactively, in already released versions!). And they'll come here and say "hey distutils-sig, can solve this problem once and for all?". And we'll be like... uh, fixing this would take what, <5 lines of new code in pip? Kinda hard to argue with that.
So... it'll be configurable, one way or another.
And if it's configurable... then the question is about whether the default configuration: is it srcdir on sys.path ("opt-out"), or srcdir not on sys.path ("opt-in"). And it seems to me that in this case, all the standard criteria say it should be opt-in.
If it's opt-in, then everyone using build backends distributed on PyPI -- which we expect to be the vast majority of project -- never have to think about it and it does the right thing, with no risk of collisions or anything. In fact the only people who have to think about it are the ones implementing in-tree backends, and if you're already like, writing a whole backend and configuring your pyproject.toml to invoke it, then asking you to add one more line of configuration is really not a big deal.
OTOH if it's opt-out, then it becomes Yet Another Bad Packaging Default, where conscientious package authors will fret about the risk of collisions and write blog posts about how every project needs to make sure to opt-out of this as a Best Practice, and I am so, so tired of those.
-n
[1] https://blog.ionelmc.ro/2014/05/25/python-packaging/#the-structure
-- Nathaniel J. Smith -- https://vorpus.org
xoviat wrote:
I agree with Nick that exceptions are the way to do things in Python
I don't agree that exceptions are the way to do "things" in general. They're the way to so *some* things. The question is whether the thing we're talking about is one of those things.
The reasoning behind this is that Python has in general adopted this approach (Nick is right that they would have used NotImplementedError for binary operations except for performance issues)
But only because NotImplemented is an out-of-band condition for operations in general. We're not talking about a general operation here, but a very specific one that only has a couple of possible results. -- Greg
I don't agree that exceptions are the way to do "things" in general. They're the way to so *some* things. The question is whether the thing we're talking about is one of those things.
I mean how is opening a file different than attempting to build an sdist? open() could return true or false and not raise any exception. But it doesn't. If you can't open the file, you get an exception. It's the same idea except here we are asking for an sdist. We would greatly prefer to have one. And if we don't have one, then we have to do something else to handle the error. This *is* Python. Sure, it may not be a good idea, though I think it's nice most of the time. It may not even be accepted for this PEP. But, although I don't have a strong opinion about whether it *should* be in the PEP, I do have a strong opinion about whether it would be an exception to the norms of Python, and my opinion is that it would. The fact that Nick, who I think is the only core developer here, immediately jumped on this issue confirms my suspicions here. 2017-08-25 20:00 GMT-05:00 Greg Ewing <greg.ewing@canterbury.ac.nz>:
xoviat wrote:
I agree with Nick that exceptions are the way to do things in Python
I don't agree that exceptions are the way to do "things" in general. They're the way to so *some* things. The question is whether the thing we're talking about is one of those things.
The reasoning behind this is that Python has in general adopted this
approach (Nick is right that they would have used NotImplementedError for binary operations except for performance issues)
But only because NotImplemented is an out-of-band condition for operations in general. We're not talking about a general operation here, but a very specific one that only has a couple of possible results.
-- Greg
_______________________________________________ Distutils-SIG maillist - Distutils-SIG@python.org https://mail.python.org/mailman/listinfo/distutils-sig
xoviat wrote:
I mean how is opening a file different than attempting to build an sdist?
1. Opening a file is a very common operation. 2. Most file opens are expected to succeed, and if one doesn't, the appropriate place to deal with that is almost never at the site of the open call, but somewhere further up. In contrast, there's probably only about one place in any given frontend where the backend's build_sdist method gets invoked, and it's unlikely the ability to let a not-implemented exception bubble up from that point would be of great use. -- Greg
On 26 August 2017 at 02:17, xoviat <xoviat@gmail.com> wrote:
The fact that Nick, who I think is the only core developer here, immediately jumped on this issue confirms my suspicions here.
*ahem* Donald and myself are both core devs too. And possibly others, I honestly don't know. (Not that I think it makes much difference - arguments should be taken on their merit, not on who made them). Paul
Donald Stufft wrote:
The thing being bubbled up is a backend accidentally calling an API that has yet to be implemented (an error that should be reported) being bubbled up and erroneously handled as the backend reporting it doesn't support making a sdist (not an error, son no traceback).
To my way of thinking, the only justification for using an exception instead of a return value as part of a protocol between a caller and a callee is if you need some kind of out-of-band return value. For example, __next__ can't use a return value to signal end of iteration, because any value could be a legitimate return value. That doesn't apply here, so there's no reason to use an exception. -- Greg
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
Nathaniel: What do you think of the proposal regarding DistutilsUnsupportedOperation? 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
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
Nathaniel Smith wrote:
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.
I don't think there's anything wrong with NotImplemented triggering different actions in different circumstances. It already has very context-dependent "fall back to something else" semantics for binary operators. -- Greg
On Thu, Aug 24, 2017 at 6:11 AM, Thomas Kluyver <thomas@kluyver.me.uk> wrote:
Nathaniel seems to be busy with other things at the moment, so I hope he won't mind me passing on this list of things he'd like to resolve with the draft PEP. I'll quote his comments and put my responses inline.
More like taking a break for mental health reasons, really, but I've been meaning to get back to it -- thanks for the nudge and I don't mind your posting it at all. -n -- Nathaniel J. Smith -- https://vorpus.org
In case anyone is interested, here is a mostly correct implementation of PEP 517 based on the behavior discussed here: https://github.com/pypa/setuptools/pull/1039/files#diff-522bd9826e33902f7f58... As I said, there are a few items to be worked on still, but I really don't think that it's too complicated. 2017-08-24 19:35 GMT-05:00 Nathaniel Smith <njs@pobox.com>:
On Thu, Aug 24, 2017 at 6:11 AM, Thomas Kluyver <thomas@kluyver.me.uk> wrote:
Nathaniel seems to be busy with other things at the moment, so I hope he won't mind me passing on this list of things he'd like to resolve with the draft PEP. I'll quote his comments and put my responses inline.
More like taking a break for mental health reasons, really, but I've been meaning to get back to it -- thanks for the nudge and I don't mind your posting it at all.
-n
-- Nathaniel J. Smith -- https://vorpus.org _______________________________________________ Distutils-SIG maillist - Distutils-SIG@python.org https://mail.python.org/mailman/listinfo/distutils-sig
participants (8)
-
Donald Stufft -
Greg Ewing -
Leonardo Rochael Almeida -
Nathaniel Smith -
Nick Coghlan -
Paul Moore -
Thomas Kluyver -
xoviat