Add __all__ list to the string module

[leewz]

BPO	26809
Nosy	@birkenfeld, @rhettinger, @vadmium, @serhiy-storchaka, @leewz, @Vgr255
Files	string___all__.patch string___all___2.patch ChainMap___all__.patch ChainMap___all___2.patch

^{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:

assignee = None
closed_at = <Date 2016-06-04.19:42:17.246>
created_at = <Date 2016-04-20.11:36:22.932>
labels = ['type-bug', 'library']
title = 'Add __all__ list to the string module'
updated_at = <Date 2016-06-04.19:42:17.244>
user = 'https://github.com/leewz'

bugs.python.org fields:

activity = <Date 2016-06-04.19:42:17.244>
actor = 'python-dev'
assignee = 'none'
closed = True
closed_date = <Date 2016-06-04.19:42:17.246>
closer = 'python-dev'
components = ['Library (Lib)']
creation = <Date 2016-04-20.11:36:22.932>
creator = 'leewz'
dependencies = []
files = ['42536', '42537', '42540', '42541']
hgrepos = []
issue_num = 26809
keywords = ['patch']
message_count = 16.0
messages = ['263824', '263825', '263829', '263831', '263832', '263833', '263834', '263835', '263836', '263850', '263851', '263852', '263874', '263884', '264015', '267302']
nosy_count = 8.0
nosy_names = ['georg.brandl', 'rhettinger', 'SilentGhost', 'python-dev', 'martin.panter', 'serhiy.storchaka', 'leewz', 'abarry']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue26809'
versions = ['Python 3.5', 'Python 3.6']

[leewz]

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:
2886808#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.

[SilentGhost]

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.

[vadmium]

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.

[leewz]

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.)

[leewz]

(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).

[vadmium]

Okay looks like I was mistaken about dir(). It ignores __all__, and just gives you everything. But the star-import point is still valid :)

[Vgr255]

Patch fixes this the proper way by defining __all__ in the string module.

[SilentGhost]

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.

It was one of mine, bpo-10895. Been dead for a while now.

[Vgr255]

Thanks SilentGhost, seems my brain decided not to see uppercase names :) Attached patch adds Formatter and Template as well.

[serhiy-storchaka]

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.

[Vgr255]

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).

[leewz]

Why not both?

[vadmium]

No strong opinion on changing the name to _ChainMap, as long as it only goes into 3.6.