Rationale behind removing kwdict from keyword.py?

Hi all, I recently got a report from a user of a project of mine breaking under 2.4a1. The reason is that keyword stopped having a kwdict variable in it. Is there any rationale behind this change? I know I can always work around it, but this honestly feels, to me, like gratuitous breakage. It would be nice if things like public members (it wasn't called _kwdict, so I had no reason to assume it was off-limits) of modules in the standard library were kept stably across releases unless there is a good reason to break compatibility. While the html docs for keyword don't mention kwdict, I often simply use help(module) to look for what I need, and I saw kwdict happily listed there, without any leading _ to indicate any sense of it being private. The changes were made, according to viewCVS, in revision 1.13. I know I can use iskeyword() for my purposes, and patch accordingly. But I'd like to remind the core developers that when things go into the stdlib, people start using them. Removing stuff will _always_ break code. I'll gladly file a SF bug report against this, but I'd like not to waste anyone's time if it won't be considered. Thanks to all. Best, Fernando Perez.

In 2.3, __all__ = ["iskeyword", "kwlist"] In the absence of separate documentation, I think __all__ may be the best hint you have about what the "external API" of a module is. Jeff

Jeff Epler wrote:
In 2.3, __all__ = ["iskeyword", "kwlist"]
In the absence of separate documentation, I think __all__ may be the best hint you have about what the "external API" of a module is.
Indeed, lesson learned. In my slim defense, seeing in help(keyword): DATA __all__ = ['iskeyword', 'kwlist'] keyword = 'yield' kwdict = {'and': 1, 'assert': 1, 'break': 1, 'class': 1, 'continue': 1... kwlist = ['and', 'assert', 'break', 'class', 'continue', 'def', 'del',... I (mistakenly) assumed kwdict was safe for usage. Relying on help(), while I acknowledge may have been mistaken, is not IMHO a completely farfetched error. Oh well, live and learn :) Thanks to all for the clarifications. I'll be far more careful in the future with my usage of stuff from the stdlib. Best, f

On Jul 28, 2004, at 12:09 AM, Fernando Perez wrote:
Jeff Epler wrote:
In 2.3, __all__ = ["iskeyword", "kwlist"]
In the absence of separate documentation, I think __all__ may be the best hint you have about what the "external API" of a module is.
Indeed, lesson learned. In my slim defense, seeing in help(keyword):
DATA __all__ = ['iskeyword', 'kwlist'] keyword = 'yield' kwdict = {'and': 1, 'assert': 1, 'break': 1, 'class': 1, 'continue': 1... kwlist = ['and', 'assert', 'break', 'class', 'continue', 'def', 'del',...
I (mistakenly) assumed kwdict was safe for usage. Relying on help(), while I acknowledge may have been mistaken, is not IMHO a completely farfetched error. Oh well, live and learn :)
Thanks to all for the clarifications. I'll be far more careful in the future with my usage of stuff from the stdlib.
FWIW, I think I've made the same mistake before for the same reasons. This sounds like a bug in pydoc. If __all__ is defined, help should probably only be given for items in it (public API). -bob

Bob> This sounds like a bug in pydoc. If __all__ is defined, help Bob> should probably only be given for items in it (public API). Funny you should mention that: revision 1.92 date: 2004/06/11 04:46:12; author: montanaro; state: Exp; lines: +22 -9 Respect a module's __all__ attribute. Closes #969938. and the corresponding output since then for the keyword module looks like: NAME keyword - Keywords (from "graminit.c") FILE /Users/skip/local/lib/python2.4/keyword.py MODULE DOCS http://www.python.org/doc/current/lib/module-keyword.html DESCRIPTION This file is automatically generated; please don't muck it up! To update the symbols in this file, 'cd' to the top directory of the python source tree after building the interpreter and run: python Lib/keyword.py FUNCTIONS iskeyword = __contains__(...) x.__contains__(y) <==> y in x. DATA __all__ = ['iskeyword', 'kwlist'] kwlist = ['and', 'assert', 'break', 'class', 'continue', 'def', 'del',... while the output using Python 2.3 is NAME keyword - Keywords (from "graminit.c") FILE /Library/Frameworks/Python.framework/Versions/2.3/lib/python2.3/keyword.py DESCRIPTION This file is automatically generated; please don't muck it up! To update the symbols in this file, 'cd' to the top directory of the python source tree after building the interpreter and run: python Lib/keyword.py FUNCTIONS iskeyword = has_key(...) D.has_key(k) -> 1 if D has a key k, else 0 main() DATA __all__ = ['iskeyword', 'kwlist'] keyword = 'yield' kwdict = {'and': 1, 'assert': 1, 'break': 1, 'class': 1, 'continue': 1... kwlist = ['and', 'assert', 'break', 'class', 'continue', 'def', 'del',... Skip

Skip Montanaro wrote:
Bob> This sounds like a bug in pydoc. If __all__ is defined, help Bob> should probably only be given for items in it (public API).
Funny you should mention that:
revision 1.92 date: 2004/06/11 04:46:12; author: montanaro; state: Exp; lines: +22 -9 Respect a module's __all__ attribute. Closes #969938.
and the corresponding output since then for the keyword module looks like:
[...] Aha! Maybe I'm not a complete idiot after all :) Glad to see this was fixed. It's always a joy to see the time machine in action, even when operated by someone other than Guido. Best, f

Relying on help(), while I acknowledge may have been mistaken, is not IMHO a completely farfetched error.
Maybe help() should only show info for things mentioned in __all__, if the module has one, unless told otherwise? Greg Ewing, Computer Science Dept, +--------------------------------------+ University of Canterbury, | A citizen of NewZealandCorp, a | Christchurch, New Zealand | wholly-owned subsidiary of USA Inc. | greg@cosc.canterbury.ac.nz +--------------------------------------+

>> Relying on help(), while I acknowledge may have been mistaken, is not >> IMHO a completely farfetched error. Greg> Maybe help() should only show info for things mentioned in Greg> __all__, if the module has one, unless told otherwise? It does, as a side-effect of the pydoc patch I referred to in an earlier message. Skip

[Fernando Perez]
I recently got a report from a user of a project of mine breaking under 2.4a1. The reason is that keyword stopped having a kwdict variable in it. Is there any rationale behind this change?
Looking at the code, because keyword.py no longer uses a dictionary (Raymond changed it to use a set), and kwdict wasn't part of keyword's public API so there was no reason to avoid changing this.
I know I can always work around it, but this honestly feels, to me, like gratuitous breakage.
It would be nice if things like public members (it wasn't called _kwdict, so I had no reason to assume it was off-limits) ...
You actually had two reasons, and strong ones. As you noted later, it wasn't part of keyword's documented interface, and you *always* act at your own risk when you go beyond the docs. The second reason is that 'kwdict' didn't appear in the keyword.__all__ list. As the docs for Python's import statement say: The public names defined by a module are determined by checking the module's namespace for a variable named __all__; if defined, it must be a sequence of strings which are names defined or imported by that module. The names given in __all__ are all considered public and are required to exist. If __all__ is not defined, the set of public names includes all names found in the module's namespace which do not begin with an underscore character ("_"). __all__ should contain the entire public API. It is intended to avoid accidentally exporting items that are not part of the API (such as library modules which were imported and used within the module). Note that it's very common in older modules (which keyword.py is) to define the newer __all__ rather than risk breaking code by massive renaming of variables to stick an underscore in front of them.
... While the html docs for keyword don't mention kwdict, I often simply use help(module) to look for what I need, and I saw kwdict happily listed there, without any leading _ to indicate any sense of it being private.
You should stop doing that, then -- this is the kind of trouble it leads to, and you'll only get more of it. help(keyword) did advertise the keyword.__all__ value in 2.3.4: ... DATA __all__ = ['iskeyword', 'kwlist'] keyword = 'yield' kwdict = {'and': 1, 'assert': 1, 'break': 1, 'class': 1, 'continue': 1... kwlist = ['and', 'assert', 'break', 'class', 'continue', 'def', 'del',... Since 'keyword' and 'kwdict' weren't in __all__, you couldn't rely on either of them; and, indeed, they're both gone in 2.4, while keyword.__all__ hasn't changed.
The changes were made, according to viewCVS, in revision 1.13. I know I can use iskeyword() for my purposes, and patch accordingly. But I'd like to remind the core developers that when things go into the stdlib, people start using them. Removing stuff will _always_ break code.
The developers were careful to keep the public API in this case exactly the same.
I'll gladly file a SF bug report against this, but I'd like not to waste anyone's time if it won't be considered.
Sorry, no, I would not consider this one.

Tim Peters wrote:
Note that it's very common in older modules (which keyword.py is) to define the newer __all__ rather than risk breaking code by massive renaming of variables to stick an underscore in front of them.
Fair enough. For a long time I'd gotten used to assuming that the lack of a leading _ was a safe sign of 'publicness' (that's probably not a word :), and didn't really rely too much on __all__. Lesson learned... (I wouldn't be surprised to find I'm not the only one with this bad habit) Thanks, I already committed a patch for my stuff anyway, since there's no point in making my users unhappy. Best regards, f

Fernando Perez <fperez528@yahoo.com> writes:
Tim Peters wrote:
Note that it's very common in older modules (which keyword.py is) to define the newer __all__ rather than risk breaking code by massive renaming of variables to stick an underscore in front of them.
Fair enough. For a long time I'd gotten used to assuming that the lack of a leading _ was a safe sign of 'publicness' (that's probably not a word :), and didn't really rely too much on __all__. Lesson learned... (I wouldn't be surprised to find I'm not the only one with this bad habit)
Well, not entirely your fault, I'd say. If kwdict wasn't part of the public API of the module, it was arguably a bug in all previous versions of the module that it *didn't* start with an underscore. Fortunately, this is shallow and easy to mend, but small random breakages like still to be avoided. Cheers, mwh -- well, take it from an old hand: the only reason it would be easier to program in C is that you can't easily express complex problems in C, so you don't. -- Erik Naggum, comp.lang.lisp
participants (7)
-
Bob Ippolito
-
Fernando Perez
-
Greg Ewing
-
Jeff Epler
-
Michael Hudson
-
Skip Montanaro
-
Tim Peters