New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add __all__ list to the string module #70996
Comments
I don't know if this kind of thing matters, but You can see in the above commit that |
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. |
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 bpo-23883 (adding to __all__ in various modules). This case is the opposite: we want to limit the exported names. |
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
(Well, not exactly the opposite. Same purpose.) |
(Never mind that last bit. I see it now.)
How do I get this behavior for |
Okay looks like I was mistaken about dir(). It ignores __all__, and just gives you everything. But the star-import point is still valid :) |
Patch fixes this the proper way by defining __all__ in the string module. |
It was one of mine, bpo-10895. Been dead for a while now. |
Thanks SilentGhost, seems my brain decided not to see uppercase names :) Attached patch adds Formatter and Template as well. |
So I think it is preferable to use this technique for ChainMap. |
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). |
Why not both? |
No strong opinion on changing the name to _ChainMap, as long as it only goes into 3.6. |
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). |
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 bpo-23883 patches to 3.6. I certainly think backporting __all__ is reasonable. |
string
exposes ChainMap from collections
New changeset 8136f9623d7f by Zachary Ware in branch '3.5': New changeset 21ae58b77924 by Zachary Ware in branch 'default': |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: