[Python-Dev] Convention on functions that shadow existing stdlib functions (original) (raw)

Terry Reedy tjreedy at udel.edu
Wed Jul 27 19:07:24 CEST 2011

On 7/27/2011 9:24 AM, Antoine Pitrou wrote:

On Wed, 27 Jul 2011 16:14:40 +0300 Eli Bendersky<eliben at gmail.com> wrote:

Will it take long for newbie code to appear with the test.support version? Not to mention that grepping code that imports the "unlink" function directly doesn't reveal which one is being used.

I think this is troublesome. I think at least two separate actions are required here: 1. In the documentation of test.support mention explicitly that it's code for CPython's internal use only, and these APIs aren't guaranteed to be stable. There is a top-level note at http://docs.python.org/dev/library/test.html, but it won't be visible by people who arrive at an anchor point.

I think the 'Note' (gray box), not a 'Warning' (red box) should be repeated at the top of the test.support section. Or, or in addition, footnote each entry (with same number jumping to same footnote, if this can be done): "test.support.verbose(1). Seeing every entry decorated with the same footnote number should indicate there there is something unusual, and that we really mean it.

I think officially documenting test.support is a mistake. There is no guarantee that APIs are stable or will even continue to exist. Docstrings are sufficient for own our purposes.

Maybe sufficient for you, but if I am to learn and use this stuff, I need a proper listing. Individual docstrings require that you know the object exists and its name. Any help(module) listing is harder for me to use than the doc chapter.

2. Some functions like unlink and rmtree are obviously redundant, and shadow frequently used Python stdlib functions, so I would either kill them completely or at least rename them appropriately. They are not redundant, since they provide slightly different semantics (for example, they silence errors that happen when the path doesn't exist).

rmtree has an ignore_errors=False parameter "ignore_errors is true, errors resulting from failed removals will be ignored; ". If that does not ignore enough errors, then perhaps it should be changed.

os.unlink is an alias for os.remove. The doc for the latter says error if path is a directory or a file in use on Windows (but not *nix). Neither should be case in test uses. Doc does not specify errors if file not exist or other conditions (inadequate permission.

Being thin wrappers, a quiet param is not allowed in os.remove/unlink, though I can imagine others wanting a quieted version for removal of files whose creation might have failed. In anycase, naming the quieted version in test.support unlink_quiet or q_remove or something would make its reason-for-being and use clearer.

Side note: test.support.import_fresh_module typo. /is/if/ in "This function will raise unittest.SkipTest is the named module cannot be imported."

-- Terry Jan Reedy

More information about the Python-Dev mailing list