Now three different flag are used to denote the same behavior. followlinks is used in os.(f)walk, symlinks is used in shutil functions (with opposite meaning), and follow_symlinks is just added to many os functions. Unfortunately, symlinks, like followlinks, is used prior to 3.3. But follow_symlinks appeared only in 3.3 (besides, it's too long a name for this frequently used parameter) and it can be unified with one of the earlier spelling. Replacing follow_symlinks to followlinks is simpler than to symlinks.

Yeah, would be nice if that was consistent.

I assert that "followlinks" and "symlinks" are both bad names.  I dislike "followlinks" because "links" is ambiguous; both hard links and soft links are "links", but it's only modifying behavior regarding one of them.  Also, it's not PEP-8-compliant (which we can forgive because I'm pretty sure it predates PEP 8).  "symlinks" is far worse, because it's so ambiguous--quick, what does "symlinks=False" mean?  Examine symlinks, or follow them?

I agree that we can't rename "followlinks" and "symlinks" in 3.x.  All we can do for now is move forward.  At the same time I refused to be shackled by misguided old nomenclature.

So, certainly, I don't want to see "follow_symlinks" changed.  True story: the reason I started writing the patch for #14626 was so I could make sure it used the name "follow_symlinks".  I was dead certain Serhiy would use one of the existing names ;-)


If you really really want this to happen, you'll have to get Georg's permission--and the sooner the better.  Already I suspect it is too late.  If it ships in 3.3 it will absolutely be too late.


I suggest another approach: add a redundant "follow_symlinks" argument to os.walk, os.fwalk, and the shutil functions.  Prefer the new name in documentation but document the presence of the old one.  Throw an exception if both are specified in an invocation.  We could do that in 3.4.

> Also, it's not PEP-8-compliant (which we can forgive because I'm pretty sure it predates PEP 8).

I don't see how this is contrary to PEP 8. PEP 8 says nothing about the
names of function arguments.

> Already I suspect it is too late.  If it ships in 3.3 it will
> absolutely be too late.

Unfortunately, too little time has passed between #14626 accepting and
shipping of 3.3b1. In any case, with accepted #14626 better than without
it.

> I suggest another approach: add a redundant "follow_symlinks" argument
> to os.walk, os.fwalk, and the shutil functions.

This is a bad solution, and I do not think that the benefits would
outweigh the disadvantages. I am -0.

> I don't see how this is contrary to PEP 8. PEP 8 says nothing about
> the names of function arguments.

Certainly it says *something*:

http://www.python.org/dev/peps/pep-0008/#function-and-method-arguments

But I grant you, it only specifically addresses "self", "cls", and function argument names that clash with keywords.

I therefore suggest that function arguments are most similar to "method names and instance variables"--after all, they *are* instance variables.  And "method names and instance variables" say "Use the function naming rules".  And function names "should be lowercase, with words separated by underscores as necessary to improve readability."

"followlinks" is comprised of two words but they are not separated by underscores.

(We can also reason by process of elimination: function arguments are wholly dissimilar to constants, package/module names, classes, and exception.  All the remaining types of identifiers in Python follow the above rule.)

It might actually be nice to clarify PEP 8 on this.


> Unfortunately, too little time has passed between #14626 accepting
> and shipping of 3.3b1.

I am genuinely sorry about that--but #14626 just wasn't ready earlier.  I'm glad you think it's an improvement--we can certainly agree on that!

Although I do dislike how long it is, I think I agree with larry on this one that follow_symlinks is the cleanest choice.  And we are post feature-freeze, so I think changing it should be done only if there is a strong reason.

IMO adding follow_symlinks to the functions currently supporting symlinks/followlinks is a bugfix.  Such a patch would be ok to go into 3.3.

> I dislike "followlinks" because "links" is ambiguous; both hard links
> and soft links are "links", but it's only modifying behavior regarding
> one of them.

Technically, in Unix world any file is a hard link. It is impossible to
distinguish a hard link from the "original", they are completely equal.
So I don't think that there will be a ambiguity, what links are implied,
especially, if the documentation is clearly says "symbolic links".

> I therefore suggest that function arguments are most similar to "method names and instance variables"--after all, they *are* instance variables.

Technically, they are local variables.

> "followlinks" is comprised of two words but they are not separated by underscores.

As "getgrouplist" or "sendfile". What about such argument names as
"filename" and "newline"? If being a consistent in that, then it must be
"follow_symbolic_links". And "source_directory_file_descriptor" instead
"src_dir_fd".

> It might actually be nice to clarify PEP 8 on this.

I agree.

> I'm glad you think it's an improvement--we can certainly agree on that!

Of course. This is the issue, for the solution of which I registered
here. All other issues and patches were just distractions and
practice. ;-)  I am grateful to you.

> IMO adding follow_symlinks to the functions currently supporting symlinks/followlinks is a bugfix.  Such a patch would be ok to go into 3.3.

Here is an other patch, that implements Larry's suggestion about
renaming followlinks in (f)walk to follow_symlinks (with keeping old
name as alias). I hope native speakers corrected me in docs. However, I
don't think that this is the best solution (but it better than nothing).

> > after all, they *are* instance variables.
>  Technically, they are local variables.

Yeah, tbh I was thinking "instance of an invocation" here.  But PEP 8 probably means "instance of a class" here.

Still, there are three classes of naming things in Python: all uppercase (constants), CapCase (class and exception names), and everything else (lowercase with underscores).  I suggest that arguments and local variables belong in the latter camp.  At no point does PEP 8 suggest naming anything in lowercase without underscores.

> As "getgrouplist" or "sendfile". What about such argument names as
> "filename" and "newline"? If being a consistent in that, then it
> must be "follow_symbolic_links". And
> "source_directory_file_descriptor" instead "src_dir_fd".

You are muddling the issue--PEP 8 makes no ruling on abbreviations.

Personally, I prefer not to use abbreviations where possible.  I've found over the years that they are painfully ambiguous--I can never remember what abbreviation I used.  ("control" becomes... "ctl"? "cntl"?)  MvL also makes a good point that abbreviations are a hinderance to people who speak little or no English.

That said, naming things after their POSIX counterparts has to be okay, and abbreviations are occasionally called for when they are widely understood / have a precedent in the library.  So with every example you cite I prefer the extant name over any counterproposal you explicitly suggested.

And here is a patch, that replaces "symlinks" arguments in shutil by
"follow_symlinks" (with keeping old name if it exists before 3.3). I
very hope native speakers corrected me in docs.

Here is updated patches. The type of exception thrown when specifying
mutually exclusive arguments changed from ValueError to TypeError (in
accordance with the raised in similar conflicts exceptions). Slightly
modified documentation. Taken into account some of Larry's comments.

Only os.walk had followlinks and shutil.copytree had symlinks before
3.3. So these are the only functions preserved the old arguments.
os.fwalk and another five public functions from shutil got a new
argument only in 3.3, for them it just renamed (may be to make them
keyword-only?). Larry worrying, isn't it too late? But it would be
strange to have in 3.3 new and immediately obsolete arguments. Moreover
the support for the two arguments complicate and slow down the code. If
you postpone it to 3.4, there will not be another solution, but now we
can do less mess.

storchaka: I can (finally!) spend some time reviewing patches today.  Which ones do I look at?  I'm guessing just the last two, "followlinks-to-follow_symlinks-2.patch" and "symlinks-to-follow_symlinks-2.patch".  But I wanted to confirm before I got knee-deep in it.

Bad news: Fearless Leader (Georg) just told me on #python-dev that he's had a change of heart and doesn't want this in 3.3.  I've marked the bug 3.4, and there's no rush on doing this work--we can't check it in until the 3.4 branch exists.  Sorry, Serhiy :(

Georg just clarified: we can just change the parameter names for new APIs.  It's the deprecation / new parameter stuff we can't do for 3.3.  So cut a (much) simpler patch and let's get it in right quick.

Here is an updated patches (with index 3). No aliased parameters, only
new parameters renamed. Some minor errors have fixed.

Serihy: for the "followlinks" patch, how about we plan ahead: I give you feedback for just the code (if there is any), and then I take over the patch and do the doc rewrite that I will find irresistable.

Storchaka: change of plan.  Since doc changes are much lower risk than code changes, how about we go with your existing patch and I'll fix up the docs separately.

So, please make the relevant code changes I proposed on Rietveld (adding "*," everywhere was it iirc) and resubmit and we can get that in.  I'll give you a code-only review of the other patch too--I'll start on that now.

> So, please make the relevant code changes I proposed on Rietveld (adding "*," everywhere was it iirc) and resubmit and we can get that in.  I'll give you a code-only review of the other patch too--I'll start on that now.

Here is a changed patches.

Personally, I doubt that followlinks-to-follow_symlinks-4.patch is
better than followlinks-to-follow_symlinks-3.patch. Also my concern is
the incompatibility of the arguments of os.walk and os.fwalk.

What's the urge to make parameters keyword-only?
Also, copyfile() shouldn't change signature.

> What's the urge to make parameters keyword-only?

If anyone uses these functions with a new "symlinks" parameter in 3.3b1,
he will get loud error (follow_symlinks == not symlinks).

> Also, copyfile() shouldn't change signature.

Why not?

> > Also, copyfile() shouldn't change signature.

> Why not?

Seems like I have misread the docs. Apparently the symlinks parameter was added in 3.3.

> What's the urge to make parameters keyword-only?

I suggest that boolean parameters are best made keyword-only.  Otherwise you have mystery meat like

shutil.copyfile("src", "dst", True)

Also, all the extant uses of "follow_symlinks" in os are keyword-only, and as long as it's a new parameter it'd be nice to keep it consistent.

The .4 patches both LGTM, please commit!

> The .4 patches both LGTM, please commit!

You're the one with commit rights here ;)

New changeset 758a9023d836 by Larry Hastings in branch 'default':
Issue #15202: Consistently use the name "follow_symlinks" for
http://hg.python.org/cpython/rev/758a9023d836

There are versionadded notes in the doc that still use the old names (e.g. symlinks for shutil.copyfile).

Sorry; the patch didn't apply cleanly, and it looks like I bungled doing it manually.  Fixing now.

New changeset e26113f17309 by Larry Hastings in branch 'default':
Issue #15202: Additional documentation fixes inadvertently omitted
http://hg.python.org/cpython/rev/e26113f17309