| 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-4db7f78c8ac9907c7ad6231730d7900c You 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 of Issue 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 of Issue 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 committed Issue 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 changeset 8136f9623d7f by Zachary Ware in branch '3.5': Issue #26809: Add __all__ to string module. Patch by Emanuel Barry https://hg.python.org/cpython/rev/8136f9623d7f New changeset 21ae58b77924 by Zachary Ware in branch 'default': Closes #26809: Merge with 3.5 https://hg.python.org/cpython/rev/21ae58b77924 |
|
|