Opened 9 years ago
Closed 9 years ago
#3588 enhancement closed fixed (fixed)
Convenience method for repeated FilePath.parent()
Reported by: | Jonathan Lange | Owned by: | |
---|---|---|---|
Priority: | normal | Milestone: | |
Component: | core | Keywords: | |
Cc: | Branch: |
branches/parents-3588
branch-diff, diff-cov, branch-cov, buildbot |
|
Author: | glyph |
Description
It would be nice if FilePath
let you do something like:
path = FilePath(__file__).parent(3)
where the above was equivalent to:
path = FilePath(__file__).parent().parent().parent()
Having a differently named function would be OK too.
Attachments (1)
Change History (17)
comment:1 Changed 9 years ago by
comment:2 Changed 9 years ago by
I could definitely use that. Your suggestion implies that FilePath(x).parents()[0] === FilePath(x), which is probably the right API but a little confusing given the name.
comment:3 follow-up: 4 Changed 9 years ago by
Hmm. I think I'd actually prefer it if FilePath(x).parents()[0] == FilePath(x).parent()
. This mirrors the behavior of segmentsFrom
, too, in your request for isParent
(you don't want isParentOrSelf()
, right?) Why do you say it's the right behavior to include the path itself?
comment:4 follow-up: 5 Changed 9 years ago by
Replying to glyph:
Hmm. I think I'd actually prefer it if
FilePath(x).parents()[0] == FilePath(x).parent()
. This mirrors the behavior ofsegmentsFrom
, too, in your request forisParent
(you don't wantisParentOrSelf()
, right?) Why do you say it's the right behavior to include the path itself?
Because in the cases where I want to use it, I actually want to iterate over the path and all of its ancestors -- I'm looking for something.
jml
comment:5 Changed 9 years ago by
Replying to jml:
Because in the cases where I want to use it, I actually want to iterate over the path and all of its ancestors -- I'm looking for something.
I think in those cases, the extra verbosity of "chain([it], it.parents())
" is useful for clarity's sake.
It occurs to me that list(it.parents())[2]
is also pretty easy too; it can easily be a generator.
To strengthen my argument on these points I shall provide a patch.
Changed 9 years ago by
Attachment: | parents.patch added |
---|
A patch which implements this, since it's pretty simple.
comment:7 follow-up: 9 Changed 9 years ago by
Keywords: | review removed |
---|---|
Owner: | set to Glyph |
Hey glyph,
The patch looks good -- thanks for getting to it so quickly. I've got a couple of minor comments:
- Isn't
assertEqual
preferred overassertEquals
? - I think the test would be clearer if it used more literals and less algorithm.
Thanks, jml
comment:8 Changed 9 years ago by
Author: | → glyph |
---|---|
Branch: | → branches/parents-3588 |
(In [25740]) Branching to 'parents-3588'
comment:9 Changed 9 years ago by
Replying to jml:
Hey glyph,
The patch looks good -- thanks for getting to it so quickly. I've got a couple of minor comments:
- Isn't
assertEqual
preferred overassertEquals
?
If you say so. I don't remember anything like that. Is this written down anywhere?
- I think the test would be clearer if it used more literals and less algorithm.
This is pretty vague, but I do think it's pretty crappy that all the variables are one letter. So let me fix that.
comment:10 Changed 9 years ago by
Status: | new → assigned |
---|
comment:11 Changed 9 years ago by
Keywords: | review added |
---|---|
Owner: | Glyph deleted |
Status: | assigned → new |
I hope I got the gist of "more literals"; I couldn't really use "less algorithm", having previously used precisely the correct amount :).
comment:12 Changed 9 years ago by
Keywords: | review removed |
---|---|
Owner: | set to Glyph |
Looks good to me. Please merge.
I realize now that my comment was too cryptic -- sorry. Here's what I meant. Instead of:
def test_parents(self): L = [] pathobj = self.path.child("a").child("b").child("c") fullpath = pathobj.path lastpath = fullpath thispath = os.path.dirname(fullpath) while lastpath != self.root.path: L.append(thispath) lastpath = thispath thispath = os.path.dirname(thispath) self.assertEqual([x.path for x in pathobj.parents()], L)
Try something like:
def test_parents(self): pathobj = self.path.child("a").child("b").child("c") expectedParents = [ self.path.child('a'), self.path.child('a').child('b'), ] self.assertEqual( [x.path for x in pathobj.parents()], [x.path for x in expectedParents])
I think the second example makes the expected behaviour of the method clearer. That said, it's your call, and I'm happy for you to merge your patch as is.
Thanks, and sorry about the roundtripping.
jml
comment:13 Changed 9 years ago by
Also, regarding assertEqual
vs assertEquals
, I don't know whether one is preferred over the other -- that's why I asked the question. I have memories of being corrected on this in the past, but I can't be sure.
comment:14 Changed 9 years ago by
I agree that your second example reads more cleanly. Unfortunately the test is incorrect :). .parents()
returns path objects all the way back up to the root, and those have to be determined algorithmically.
comment:15 Changed 9 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:16 Changed 7 years ago by
Owner: | Glyph deleted |
---|
How about FilePath(file).parents()[3]?
This could also address #3589 with a convenient idiom rather than a specific method:
if possibleParent in possibleChild.parents()
...