Hello, Python 3.7 removes the undocumented internal import `os.errno`. We consider that a an implementation detail, which can be changed *without notice* even in a bugfix release. Projects that depend on it are incorrect and should be fixed. On bpo-33666, there's a debate on whether the removal should be mentioned in release notes, on the grounds that it broke some projects, is used in quire a few tutorials/books/examples, and it's been working since Python 2.5 or so. But here's the thing: the more I think about this, the less I consider `os.errno` as "undocumented". Here's what I consider a reasonable path a programmer might go through: # Where do I find errno values? # Maybe it's in `os`, like all other basic platform bindings?
Even more serious: # Hmm, this old example on some website tells me to use `os.errno`. # It's the first time I'm hear about that, so I'll be extra careful;
As you can see, the built-in documentation does not contain *any* warnings against using `os.errno`. You might think the fact that it's called "errno" and not "os.errno" is a red flag, but it's not, really -- "os.path" is (on my system) named "posixpath", yet "os.path" is the name to use. While most people might prefer searching docs.python org to `help()`, editors are getting better and better to presenting introspection and the built-in docs, so more and more people are preferring `pydoc`-ish docs to going online. I don't think we can reasonably expect people who used built-in help as above to go back and check that Python's official docs *do not* contain `os.errno`. Effectively, while `os.errno` is not very *discoverable* using official docs alone, I don't think calling it *undocumented* is fair. So, removing it without notice is not very friendly to our users. Is that reasoning sound? Should our policy on removing internal imports take that into account?
Agree, but...
`pydoc os` shows: This exports: - all functions from posix or nt, e.g. unlink, stat, etc. - os.path is either posixpath or ntpath - os.name is either 'posix' or 'nt' - os.curdir is a string representing the current directory (always '.') - os.pardir is a string representing the parent directory (always '..') - os.sep is the (or a most common) pathname separator ('/' or '\\') - os.extsep is the extension separator (always '.') - os.altsep is the alternate pathname separator (None or '/') - os.pathsep is the component separator used in $PATH etc - os.linesep is the line separator in text files ('\r' or '\n' or '\r\n') - os.defpath is the default search path for executables - os.devnull is the file path of the null device ('/dev/null', etc.) So I think calling `os.errno` as *undocumented* is fair.
So, removing it without notice is not very friendly to our users.
Is that reasoning sound?
Depending on such subimports is common pitfall all experienced Python programmer knows. Of course, it's not very friendly, but a pitfall. But I don't think it's enough reason to call it *documented*.
Should our policy on removing internal imports take that into account?
Maybe, we should use `_` prefix for all new private subimports. I don't have any idea having better "maintainability : user friendly" ratio than it. Third party linter may be able to notice warning for using private subimports in user code. Regards, -- INADA Naoki <songofacandy@gmail.com>
On Tue, May 29, 2018 at 8:06 PM, Petr Viktorin <encukou@gmail.com> wrote:
At no point did you show that it's *documented*. What you've shown is that it's an unadorned name. There are a lot of those around:
If people are attempting "from os import errno", they need to learn to type "import errno" instead. While it's nice to hide those modules with "import errno as _errno", failure to do so is a bug, not an API feature. The change might break code, but ANY change might break code: https://xkcd.com/1172/ ChrisA
On Tue, May 29, 2018 at 12:06:42PM +0200, Petr Viktorin wrote:
PEP 8 makes it clear that imports are implementation details unless explicitly documented otherwise: Imported names should always be considered an implementation detail. Other modules must not rely on indirect access to such imported names unless they are an explicitly documented part of the containing module's API, such as os.path or a package's __init__ module that exposes functionality from submodules. This decision dates back to 2013: https://mail.python.org/pipermail/python-dev/2013-July/127284.html so it has been around for a while, long enough that linters ought to be enforcing it, and people ought to know about it. If not, that's a failure of the linter and/or the coder.
I don't see why there should be a debate about mentioning it in the release notes. There's no harm in adding a few lines: "os.errno is a non-public implementation detail, as described in PEP 8, and has been removed. Import the errno module instead." Being an implementation detail, we aren't *required* to do so, but given that (so you say) a few tutorials etc use it, I think it is the kind thing to do.
All of this is reasonable, and I'm sympathetic, *except* that it is officially documented policy that imports are implementation details unless otherwise stated. If os.errno was gone and there was no easy fix, I think we could be merciful and rethink the decision. But there is an easy fix: import errno directly instead. And maybe this will encourage linters to flag this risky usage, if they aren't already doing so.
As you can see, the built-in documentation does not contain *any* warnings against using `os.errno`.
It doesn't need to. And of course, help(os.errno) *cannot* warn about os.errno, since it only receives the errno module as argument, not the expression you used to pass it. -- Steve
(Apologies if you received an empty/unfinished e-mail here earlier; I hit Send by mistake.) On 05/29/18 13:57, Steven D'Aprano wrote:
I agree that it's technically well within our rights to remove it without notice. But ... PEP8? A style guide defines what is a CPython implementation detail? That's not a place to point to while saying "told you so!" -- perhaps "sorry for the inconvenience" would be more appropriate :)
How do linters find out what's an internal import, and what's correct usage (like os.path)?
Indeed. That's unfortunate, but it is a reason for us to be careful, or perhaps find/document a better policy for handling these. I'm not looking for evidence to justify the changes; I'm looking for ways to be more friendly to our users -- most of whom have not read all of the docs.
https://www.python.org/dev/peps/pep-0008/#public-and-internal-interfaces "Any backwards compatibility guarantees apply only to public interfaces." "All undocumented interfaces should be assumed to be internal." "Imported names should always be considered an implementation detail. Other modules must not rely on indirect access to such imported names unless they are an explicitly documented part of the containing module's API, such as os.path or a package's __init__ module that exposes functionality from submodules."
And maybe this will encourage linters to flag this risky usage, if they aren't already doing so.
How do linters find out what's an internal import, and what's correct usage (like os.path)?
It is `correct usage` only when it is in __all__.
Regards, -- INADA Naoki <songofacandy@gmail.com>
On 29 May 2018 at 20:06, Petr Viktorin <encukou@gmail.com> wrote:
Is that reasoning sound? Should our policy on removing internal imports take that into account?
As Steven noted, the normative answer to this is in PEP 8: https://www.python.org/dev/peps/pep-0008/#public-and-internal-interfaces Since `os.errno` is a transitive import, is not included in `os.__all__`, and isn't documented in the library reference, it's considered an implementation detail that can be removed without a deprecation period. That said, it should still be mentioned in the "Porting to Python 3.7" section of the What's New guide, with the fix being to switch to "import errno" in any affected code. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
29.05.18 13:06, Petr Viktorin пише:
Unlike to os.path, which is explicitly documented and included in __all__, os.abc, os.errno, os.sys and os.st are not documented and not included in __all__. They were always an implementations detail. os.errno was added as a side effect of issue1755179. [1]
We have removed over 100 module attributes in 3.6 [2] and over 200 module attributes in 3.7 [3]. Some of these removals broke third-part projects [4], even removals of underscored names. If we will document the removal of os.errno, we should to document the removal of at least other names for which there are known examples of breaking third-party projects, e.g. re._pattern_type, uuid._uuid_generate_time and several typing attributes. [1] https://bugs.python.org/issue1755179 [2] https://bugs.python.org/msg317881 [3] https://bugs.python.org/msg317876 [4] https://bugs.python.org/msg317998
On Tue, May 29, 2018 at 05:37:06PM +0300, Serhiy Storchaka wrote:
Yes, people will rely on undocumented features. Even when you tell them not to.
I disagree. I think that _leading underscore names can be removed without mentioning the removal. If it breaks code, too bad. Anyone using an explicitly named _private name has only themselves to blame. But non-public names *without* a leading underscore are in a grey area, since they *look* like public names. I think we can afford to be kind by documenting such removals -- especially since it may help to educate people that imported modules without a leading underscore are still considered implementation details. -- Steve
On 5/29/2018 12:09 PM, Guido van Rossum wrote:
Yes. What Steve says at the end. Let's be kind and mention this one on the release notes. But not all the others.
How about adding a general reminder. In Porting to 3.7: "Imports into a module are a private implementation detail unless otherwise noted. Private imports can be removed when not needed. (See PEP 8.) For example, the os module no longer needs the errno module, so 'import errno' was removed for 3.7. If you accessed 'errno' as 'os.errno', add 'import errno' to your code and remove the 'os.' prefix. The same applies to the 200 other unneeded imports removed in 3.7." Since this is a repeated problem, and a repeated drain on us core developers, a version of this should be repeated in every What's New. -- Terry Jan Reedy
Have we received complaints about other cases? Whenever this breaks code for which I am responsible I just blush and fix it, I don't complain. Repeating the same warning typically just causes warning fatigue rather than helping anyone. On Tue, May 29, 2018 at 3:53 PM, Terry Reedy <tjreedy@udel.edu> wrote:
-- --Guido van Rossum (python.org/~guido)
On 29/05/2018 23:58, Guido van Rossum wrote:
Maybe what we need is to add a, possibly optional, or suppressible, warning whenever the import system encounters an implicit/indirect import? If an import that is working because the package we are importing it from has imported it from elsewhere, but it is not included in the __init__ for that package, then code authors will get some warning before the breakage and any blushes will really be due. If I was getting something along the lines of: Warning: Implicit Import in MyCode.py line 16 - import of os.errno is unsupported and may stop working on any update consider direct import. Then I would try to address it at the current version. I would be happy to try to implement this and think that the performance impact should be low. -- Steve (Gadget) Barnes Any opinions in this message are my personal opinions and do not reflect those of my employer. --- This email has been checked for viruses by AVG. http://www.avg.com
On Wed, May 30, 2018 at 04:18:51AM +0000, Steve Barnes wrote:
I don't think your terminology ("implicit/indirect") is very accurate. from os import errno is as explicit as you can get.
I think you mean __all__ for the module. I'm not sure how that check would work. For a simple module, whenever you call "from module import name", the interpreter has to inspect the object it just imported, and if it is a module itself, check whether "name" is in the owning module's __all__. How would it work for packages? "from package import submodule" ought to work without a warning even if submodule isn't listed in __all__. Even for simple modules, it is prone to false positives: if "name" is documented as public, but not listed in __all__ then that would wrongly be detected as a non-public import. But most problematic, it does nothing about this case: import os os.errno I think this is best left for linters. -- Steve
On 5/29/2018 6:06 AM, Petr Viktorin wrote:
Python 3.7 removes the undocumented internal import `os.errno`.
Among a couple of hundred others, including some from idlelib.
We consider that a an implementation detail, which can be changed *without notice* even in a bugfix release.
We core developers occasionally run a linter on our code, or someone does it for us. Those of us who like clean code attend to warnings such as 'unused import'. I removed a couple of idlelib inports because of someone else running flake8.
Projects that depend on it are incorrect and should be fixed.
I think we can agree that better warning of 'use of stdlib internal import' would be great.
Since I agree that *this* removal may possibly have the greatest impact, and that the purpose of What's New is to help users, I am glad Guido spoke up to say 'do it'. See my reponse to his post for a suggested entry.
Suppose a naive beginner who confuses 'IDLE' with 'python' (they exist!) thinks "Where can I find re functions? How about 'pyshell'
!
Is that reasoning sound?
Do you claim that os.errno is more documented (by the 'official' docs and help) than idlelib.pyshell.re? -- Terry Jan Reedy
Agree, but...
`pydoc os` shows: This exports: - all functions from posix or nt, e.g. unlink, stat, etc. - os.path is either posixpath or ntpath - os.name is either 'posix' or 'nt' - os.curdir is a string representing the current directory (always '.') - os.pardir is a string representing the parent directory (always '..') - os.sep is the (or a most common) pathname separator ('/' or '\\') - os.extsep is the extension separator (always '.') - os.altsep is the alternate pathname separator (None or '/') - os.pathsep is the component separator used in $PATH etc - os.linesep is the line separator in text files ('\r' or '\n' or '\r\n') - os.defpath is the default search path for executables - os.devnull is the file path of the null device ('/dev/null', etc.) So I think calling `os.errno` as *undocumented* is fair.
So, removing it without notice is not very friendly to our users.
Is that reasoning sound?
Depending on such subimports is common pitfall all experienced Python programmer knows. Of course, it's not very friendly, but a pitfall. But I don't think it's enough reason to call it *documented*.
Should our policy on removing internal imports take that into account?
Maybe, we should use `_` prefix for all new private subimports. I don't have any idea having better "maintainability : user friendly" ratio than it. Third party linter may be able to notice warning for using private subimports in user code. Regards, -- INADA Naoki <songofacandy@gmail.com>
On Tue, May 29, 2018 at 8:06 PM, Petr Viktorin <encukou@gmail.com> wrote:
At no point did you show that it's *documented*. What you've shown is that it's an unadorned name. There are a lot of those around:
If people are attempting "from os import errno", they need to learn to type "import errno" instead. While it's nice to hide those modules with "import errno as _errno", failure to do so is a bug, not an API feature. The change might break code, but ANY change might break code: https://xkcd.com/1172/ ChrisA
On Tue, May 29, 2018 at 12:06:42PM +0200, Petr Viktorin wrote:
PEP 8 makes it clear that imports are implementation details unless explicitly documented otherwise: Imported names should always be considered an implementation detail. Other modules must not rely on indirect access to such imported names unless they are an explicitly documented part of the containing module's API, such as os.path or a package's __init__ module that exposes functionality from submodules. This decision dates back to 2013: https://mail.python.org/pipermail/python-dev/2013-July/127284.html so it has been around for a while, long enough that linters ought to be enforcing it, and people ought to know about it. If not, that's a failure of the linter and/or the coder.
I don't see why there should be a debate about mentioning it in the release notes. There's no harm in adding a few lines: "os.errno is a non-public implementation detail, as described in PEP 8, and has been removed. Import the errno module instead." Being an implementation detail, we aren't *required* to do so, but given that (so you say) a few tutorials etc use it, I think it is the kind thing to do.
All of this is reasonable, and I'm sympathetic, *except* that it is officially documented policy that imports are implementation details unless otherwise stated. If os.errno was gone and there was no easy fix, I think we could be merciful and rethink the decision. But there is an easy fix: import errno directly instead. And maybe this will encourage linters to flag this risky usage, if they aren't already doing so.
As you can see, the built-in documentation does not contain *any* warnings against using `os.errno`.
It doesn't need to. And of course, help(os.errno) *cannot* warn about os.errno, since it only receives the errno module as argument, not the expression you used to pass it. -- Steve
(Apologies if you received an empty/unfinished e-mail here earlier; I hit Send by mistake.) On 05/29/18 13:57, Steven D'Aprano wrote:
I agree that it's technically well within our rights to remove it without notice. But ... PEP8? A style guide defines what is a CPython implementation detail? That's not a place to point to while saying "told you so!" -- perhaps "sorry for the inconvenience" would be more appropriate :)
How do linters find out what's an internal import, and what's correct usage (like os.path)?
Indeed. That's unfortunate, but it is a reason for us to be careful, or perhaps find/document a better policy for handling these. I'm not looking for evidence to justify the changes; I'm looking for ways to be more friendly to our users -- most of whom have not read all of the docs.
https://www.python.org/dev/peps/pep-0008/#public-and-internal-interfaces "Any backwards compatibility guarantees apply only to public interfaces." "All undocumented interfaces should be assumed to be internal." "Imported names should always be considered an implementation detail. Other modules must not rely on indirect access to such imported names unless they are an explicitly documented part of the containing module's API, such as os.path or a package's __init__ module that exposes functionality from submodules."
And maybe this will encourage linters to flag this risky usage, if they aren't already doing so.
How do linters find out what's an internal import, and what's correct usage (like os.path)?
It is `correct usage` only when it is in __all__.
Regards, -- INADA Naoki <songofacandy@gmail.com>
On 29 May 2018 at 20:06, Petr Viktorin <encukou@gmail.com> wrote:
Is that reasoning sound? Should our policy on removing internal imports take that into account?
As Steven noted, the normative answer to this is in PEP 8: https://www.python.org/dev/peps/pep-0008/#public-and-internal-interfaces Since `os.errno` is a transitive import, is not included in `os.__all__`, and isn't documented in the library reference, it's considered an implementation detail that can be removed without a deprecation period. That said, it should still be mentioned in the "Porting to Python 3.7" section of the What's New guide, with the fix being to switch to "import errno" in any affected code. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
29.05.18 13:06, Petr Viktorin пише:
Unlike to os.path, which is explicitly documented and included in __all__, os.abc, os.errno, os.sys and os.st are not documented and not included in __all__. They were always an implementations detail. os.errno was added as a side effect of issue1755179. [1]
We have removed over 100 module attributes in 3.6 [2] and over 200 module attributes in 3.7 [3]. Some of these removals broke third-part projects [4], even removals of underscored names. If we will document the removal of os.errno, we should to document the removal of at least other names for which there are known examples of breaking third-party projects, e.g. re._pattern_type, uuid._uuid_generate_time and several typing attributes. [1] https://bugs.python.org/issue1755179 [2] https://bugs.python.org/msg317881 [3] https://bugs.python.org/msg317876 [4] https://bugs.python.org/msg317998
On Tue, May 29, 2018 at 05:37:06PM +0300, Serhiy Storchaka wrote:
Yes, people will rely on undocumented features. Even when you tell them not to.
I disagree. I think that _leading underscore names can be removed without mentioning the removal. If it breaks code, too bad. Anyone using an explicitly named _private name has only themselves to blame. But non-public names *without* a leading underscore are in a grey area, since they *look* like public names. I think we can afford to be kind by documenting such removals -- especially since it may help to educate people that imported modules without a leading underscore are still considered implementation details. -- Steve
On 5/29/2018 12:09 PM, Guido van Rossum wrote:
Yes. What Steve says at the end. Let's be kind and mention this one on the release notes. But not all the others.
How about adding a general reminder. In Porting to 3.7: "Imports into a module are a private implementation detail unless otherwise noted. Private imports can be removed when not needed. (See PEP 8.) For example, the os module no longer needs the errno module, so 'import errno' was removed for 3.7. If you accessed 'errno' as 'os.errno', add 'import errno' to your code and remove the 'os.' prefix. The same applies to the 200 other unneeded imports removed in 3.7." Since this is a repeated problem, and a repeated drain on us core developers, a version of this should be repeated in every What's New. -- Terry Jan Reedy
Have we received complaints about other cases? Whenever this breaks code for which I am responsible I just blush and fix it, I don't complain. Repeating the same warning typically just causes warning fatigue rather than helping anyone. On Tue, May 29, 2018 at 3:53 PM, Terry Reedy <tjreedy@udel.edu> wrote:
-- --Guido van Rossum (python.org/~guido)
On 29/05/2018 23:58, Guido van Rossum wrote:
Maybe what we need is to add a, possibly optional, or suppressible, warning whenever the import system encounters an implicit/indirect import? If an import that is working because the package we are importing it from has imported it from elsewhere, but it is not included in the __init__ for that package, then code authors will get some warning before the breakage and any blushes will really be due. If I was getting something along the lines of: Warning: Implicit Import in MyCode.py line 16 - import of os.errno is unsupported and may stop working on any update consider direct import. Then I would try to address it at the current version. I would be happy to try to implement this and think that the performance impact should be low. -- Steve (Gadget) Barnes Any opinions in this message are my personal opinions and do not reflect those of my employer. --- This email has been checked for viruses by AVG. http://www.avg.com
On Wed, May 30, 2018 at 04:18:51AM +0000, Steve Barnes wrote:
I don't think your terminology ("implicit/indirect") is very accurate. from os import errno is as explicit as you can get.
I think you mean __all__ for the module. I'm not sure how that check would work. For a simple module, whenever you call "from module import name", the interpreter has to inspect the object it just imported, and if it is a module itself, check whether "name" is in the owning module's __all__. How would it work for packages? "from package import submodule" ought to work without a warning even if submodule isn't listed in __all__. Even for simple modules, it is prone to false positives: if "name" is documented as public, but not listed in __all__ then that would wrongly be detected as a non-public import. But most problematic, it does nothing about this case: import os os.errno I think this is best left for linters. -- Steve
On 5/29/2018 6:06 AM, Petr Viktorin wrote:
Python 3.7 removes the undocumented internal import `os.errno`.
Among a couple of hundred others, including some from idlelib.
We consider that a an implementation detail, which can be changed *without notice* even in a bugfix release.
We core developers occasionally run a linter on our code, or someone does it for us. Those of us who like clean code attend to warnings such as 'unused import'. I removed a couple of idlelib inports because of someone else running flake8.
Projects that depend on it are incorrect and should be fixed.
I think we can agree that better warning of 'use of stdlib internal import' would be great.
Since I agree that *this* removal may possibly have the greatest impact, and that the purpose of What's New is to help users, I am glad Guido spoke up to say 'do it'. See my reponse to his post for a suggested entry.
Suppose a naive beginner who confuses 'IDLE' with 'python' (they exist!) thinks "Where can I find re functions? How about 'pyshell'
!
Is that reasoning sound?
Do you claim that os.errno is more documented (by the 'official' docs and help) than idlelib.pyshell.re? -- Terry Jan Reedy
participants (10)
-
Chris Angelico -
Guido van Rossum -
Guido van Rossum -
INADA Naoki -
Nick Coghlan -
Petr Viktorin -
Serhiy Storchaka -
Steve Barnes -
Steven D'Aprano -
Terry Reedy