classification
Title: Support os.walk(dir_fd=)
Type: enhancement Stage: patch review
Components: Versions: Python 3.4
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: larry Nosy List: Arfrever, georg.brandl, larry, rhettinger, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2012-06-25 23:39 by larry, last changed 2013-03-28 10:12 by georg.brandl. This issue is now closed.

Files
File name Uploaded Description Edit
larry.os.walk.dir_fd.1.diff larry, 2012-06-25 23:39 Patch #1 adding dir_fd to os.walk(). review
Messages (9)
msg164023 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-06-25 23:39
Adds dir_fd support to os.walk().  Just re-yields from os.fwalk(), removing the last element.

Note: Python 3.4.
msg164031 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2012-06-26 04:21
Can you elaborate on why this is needed?

The os.path.walk() API is already somewhat complex.  Additional features can make the tool less usable/learnable for most users.
msg164037 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-06-26 05:27
> Can you elaborate on why this is needed?

1. Unification with fwalk and other os functions.
2. Performance. Larry, can you use fwalk even for dir_fd is None (if fwalk exists, of cause)?
msg164040 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-06-26 06:17
Actually I think Raymond makes a good point.

Re: symmetry: tbh that's nonsense.  The reason for symmetry among functions in the os module is because they do similar things--but this is because "form follows function".  We didn't decide to decorate functions with extra parameters just so they'd look nice.

The reason you want a function to support "dir_fd" is to make it safer; using functions taking "dir_fd" and some careful programming, you can prevent some forms of timing attack.  But we can't fix os.walk to make it safe in this way--which is why we have os.fwalk in the first place!  So users of os.walk with this problem simply don't need "dir_fd"--they need os.fwalk.

Re: performance: if some people care about performance here, and this approach is faster, then those people can just call os.fwalk directly.  This approach to os.walk(dir_fd=) just calls os.fwalk--so calling it directly could only be even faster.

(This is assuming my favored implementation which just calls os.fwalk--which is simple, and leverages os.fwalk doing a proper safe job of it.  If we use my hackier previous version, I suspect it only made os.walk slower.  Of course all of this is silly microbenchmarking anyway, and I'm not sure that these fiddling implementation details of os.walk / os.fwalk contribute to their runtime cost in any significant way.)


I counter-propose adding some text to os.walk steering people to os.fwalk for "advanced usage".  That might even be appropriate for 3.3 (beta 2).  I realize they are neighbors in the documentation, but it might save at least one benighted myopic soul.

I further propose to leave this issue open for now--there's no rush, really--and see if a compelling reason for os.walk(dir_fd=) appears.  If none does, then in the fullness of time we can close this as wontfix and move on, living our lives in the dazzling sunshine of righteous truth and justice.

Your move, Serhiy ;-)

p.s. Serhiy: yes, you can call os.fwalk() with dir_fd=None.  It would be *awful* if you could not!  And actually, os.fwalk didn't even support dir_fd until... 26 hours ago.
msg164044 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-06-26 06:29
p.s. Raymond: fwiw, I think "makes it easy / hard for beginners" is only of secondary importance.  Certainly I think it's reasonable to point out in a discussion, and if the beginners are easy to accommodate then okay.  But if there was something that was a big win in most ways but made life harder for beginners, imo the beginners would have to take it on the chin.

I reflect now and then on the fascinating point you made on Radio Free Python--"Unicode is now day-0 knowledge for Python"--but I have no idea what to do about it.  Or about the larger point of accommodating beginners in the face of mounting language complexity. *shrug*

My point is, I agree that os.walk(dir_fd=) probably shouldn't happen--but that's simply because we don't need it.  Whether or not it made life easier or harder for beginners didn't really figure into my consideration.
msg164045 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-06-26 06:31
Whoops, you wrote "for most users" rather than talking about beginners.  Sorry if I was responding to a point you weren't making ;-)  I guess I assumed you were coming at this at least partially while wearing your "Python lecturer" hat.
msg164135 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-06-27 09:41
> Your move, Serhiy ;-)

It seems that you play for both sides in the last days. ;-)  I surrender. Really, I'm not interested in this feature, but as of symmetry, and this is a very weak motive.

> p.s. Serhiy: yes, you can call os.fwalk() with dir_fd=None.  It would be *awful* if you could not!  And actually, os.fwalk didn't even support dir_fd until... 26 hours ago.

See issue15200.
msg172217 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-10-06 16:30
Should this issue be closed?
msg185417 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2013-03-28 10:12
Closing (it seems everybody agreed it's not a good idea).
History
Date User Action Args
2013-03-28 10:12:04georg.brandlsetstatus: pending -> closed

nosy: + georg.brandl
messages: + msg185417

resolution: rejected
2012-10-20 20:04:44serhiy.storchakasetstatus: open -> pending
2012-10-06 16:30:54serhiy.storchakasetmessages: + msg172217
2012-06-27 09:41:02serhiy.storchakasetmessages: + msg164135
2012-06-26 06:31:35larrysetmessages: + msg164045
2012-06-26 06:29:54larrysetmessages: + msg164044
2012-06-26 06:17:30larrysetmessages: + msg164040
2012-06-26 05:27:44serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg164037
2012-06-26 04:21:30rhettingersetnosy: + rhettinger
messages: + msg164031
2012-06-26 00:03:30Arfreversetnosy: + Arfrever
2012-06-25 23:39:44larrycreate