
This issue trackerhas been migrated toGitHub, and is currentlyread-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.
Created on2016-04-20 11:36 byleewz, last changed2022-04-11 14:58 byadmin. This issue is nowclosed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| string___all__.patch | abarry,2016-04-20 13:22 | review | ||
| string___all___2.patch | abarry,2016-04-20 13:33 | review | ||
| ChainMap___all__.patch | abarry,2016-04-20 16:46 | review | ||
| ChainMap___all___2.patch | abarry,2016-04-20 16:54 | review | ||
| Messages (16) | |||
|---|---|---|---|
| msg263824 -(view) | Author: Franklin? Lee (leewz) | Date: 2016-04-20 11:36 | |
I don't know if this kind of thing matters, but `from string import ChainMap` works (imports from `collections). It's used internally by `string`. This was done when ChainMap was made official:https://github.com/python/cpython/commit/2886808d3809d69a6e9b360380080140b95df0b6#diff-4db7f78c8ac9907c7ad6231730d7900cYou can see in the above commit that `configparser` had `_ChainMap` changed to `ChainMap as _ChainMap`, but this was not done in `string`. | |||
| msg263825 -(view) | Author: SilentGhost (SilentGhost)*![]() | Date: 2016-04-20 11:48 | |
I wouldn't say it matter, "from configparser import _ChainMap" works too, but that's just how imports work in Python. There is not reason to do what you've done on purpose - ChainMap is undocumented and is clearly just a detail of implementation.The fact that ChainMap pollutes namespace when doing star import ("from string import *"), however, is unfortunate. I believe there was an issue around that was supposed to add missing __all__, but I'm not able to find that issue atm. | |||
| msg263829 -(view) | Author: Martin Panter (martin.panter)*![]() | Date: 2016-04-20 13:02 | |
I agree that the namespace pollution is a valid bug. It affects “from string import *” and dir(string). Normally, modules either define __all__ listing their exported names, or they are really careful to define non-exported names beginning with underscores (_). Judging by the line “import re as _re”, somebody once tried to use the second technique for the “string” module.Silent Ghost: perhaps you were thinking ofIssue 23883 (adding to __all__ in various modules). This case is the opposite: we want to limit the exported names. | |||
| msg263831 -(view) | Author: Franklin? Lee (leewz) | Date: 2016-04-20 13:07 | |
> The fact that ChainMap pollutes namespace when doing star import ("from string import *"), however, is unfortunate.This is what I meant by "this kind of thing". (IPython also ignores underscored names for autocomplete suggestions, unless you start with an underscore. IPython doesn't respect `__all__` by default, but that's a report for IPython's tracker.)> This case is the opposite: we want to limit the exported names.(Well, not exactly the opposite. Same purpose.) | |||
| msg263832 -(view) | Author: Franklin? Lee (leewz) | Date: 2016-04-20 13:10 | |
(Never mind that last bit. I see it now.)> I agree that the namespace pollution is a valid bug. It affects “from string import *” and dir(string).How do I get this behavior for `dir`? I get '_re' in `dir(string)`. | |||
| msg263833 -(view) | Author: Martin Panter (martin.panter)*![]() | Date: 2016-04-20 13:14 | |
Okay looks like I was mistaken about dir(). It ignores __all__, and just gives you everything. But the star-import point is still valid :) | |||
| msg263834 -(view) | Author: Anilyka Barry (abarry)*![]() | Date: 2016-04-20 13:22 | |
Patch fixes this the proper way by defining __all__ in the string module. | |||
| msg263835 -(view) | Author: SilentGhost (SilentGhost)*![]() | Date: 2016-04-20 13:26 | |
> Silent Ghost: perhaps you were thinking ofIssue 23883 (adding to __all__ in various modules). This case is the opposite: we want to limit the exported names.It was one of mine,issue 10895. Been dead for a while now. | |||
| msg263836 -(view) | Author: Anilyka Barry (abarry)*![]() | Date: 2016-04-20 13:33 | |
Thanks SilentGhost, seems my brain decided not to see uppercase names :) Attached patch adds Formatter and Template as well. | |||
| msg263850 -(view) | Author: Serhiy Storchaka (serhiy.storchaka)*![]() | Date: 2016-04-20 16:37 | |
> Judging by the line “import re as _re”, somebody once tried to use the second technique for the “string” module.So I think it is preferable to use this technique for ChainMap. | |||
| msg263851 -(view) | Author: Anilyka Barry (abarry)*![]() | Date: 2016-04-20 16:46 | |
I added an underscore in front of ChainMap, but I kept the __all__ definition because I think it should be there regardless (like every module, basically). | |||
| msg263852 -(view) | Author: Franklin? Lee (leewz) | Date: 2016-04-20 16:46 | |
Why not both? | |||
| msg263874 -(view) | Author: Martin Panter (martin.panter)*![]() | Date: 2016-04-21 04:19 | |
No strong opinion on changing the name to _ChainMap, as long as it only goes into 3.6. | |||
| msg263884 -(view) | Author: Raymond Hettinger (rhettinger)*![]() | Date: 2016-04-21 06:17 | |
ChainMap___all___2.patch looks good. I would be fine with this going into 3.5 as well (ChainMap is not part of the documented API for the string module). | |||
| msg264015 -(view) | Author: Martin Panter (martin.panter)*![]() | Date: 2016-04-22 14:03 | |
I’m not that fussed if the _ChainMap name is backported. I just thought it is safer to not do it; similar reasoning to why I only committedIssue 23883 patches to 3.6. I certainly think backporting __all__ is reasonable. | |||
| msg267302 -(view) | Author: Roundup Robot (python-dev)![]() | Date: 2016-06-04 19:42 | |
New changeset8136f9623d7f by Zachary Ware in branch '3.5':Issue#26809: Add __all__ to string module. Patch by Emanuel Barryhttps://hg.python.org/cpython/rev/8136f9623d7fNew changeset21ae58b77924 by Zachary Ware in branch 'default':Closes#26809: Merge with 3.5https://hg.python.org/cpython/rev/21ae58b77924 | |||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:58:29 | admin | set | github: 70996 |
| 2016-06-04 19:42:17 | python-dev | set | status: open -> closed nosy: +python-dev messages: +msg267302 resolution: fixed stage: patch review -> resolved |
| 2016-06-04 18:54:35 | abarry | set | title: `string` exposes ChainMap from `collections` -> Add __all__ list to the string module |
| 2016-04-26 06:13:37 | rhettinger | set | assignee:rhettinger -> |
| 2016-04-22 14:03:14 | martin.panter | set | messages: +msg264015 |
| 2016-04-21 06:17:49 | rhettinger | set | messages: +msg263884 |
| 2016-04-21 06:10:34 | rhettinger | set | assignee:rhettinger nosy: +rhettinger |
| 2016-04-21 04:19:58 | martin.panter | set | messages: +msg263874 stage: patch review |
| 2016-04-20 16:54:38 | abarry | set | files: +ChainMap___all___2.patch |
| 2016-04-20 16:46:58 | leewz | set | messages: +msg263852 |
| 2016-04-20 16:46:44 | abarry | set | files: +ChainMap___all__.patch messages: +msg263851 |
| 2016-04-20 16:37:43 | serhiy.storchaka | set | nosy: +serhiy.storchaka messages: +msg263850 |
| 2016-04-20 13:33:42 | abarry | set | files: +string___all___2.patch messages: +msg263836 |
| 2016-04-20 13:26:22 | SilentGhost | set | messages: +msg263835 |
| 2016-04-20 13:22:52 | abarry | set | files: +string___all__.patch nosy: +abarry messages: +msg263834 keywords: +patch |
| 2016-04-20 13:14:35 | martin.panter | set | messages: +msg263833 |
| 2016-04-20 13:10:46 | leewz | set | messages: +msg263832 |
| 2016-04-20 13:07:53 | leewz | set | messages: +msg263831 |
| 2016-04-20 13:02:34 | martin.panter | set | nosy: +martin.panter messages: +msg263829 |
| 2016-04-20 11:48:58 | SilentGhost | set | nosy: +georg.brandl,SilentGhost messages: +msg263825 |
| 2016-04-20 11:36:22 | leewz | create | |