`zipfile`: `zipfile.Path`’s `glob()` and `rglob()` are not documented

[calestyo]

Documentation

It seems documentation for zipfile.Path’s glob() and rglob() is missing (or maybe they're meant to be private but wrongly "exported"?!

Also, the behaviour of rglob() seems a bit unexpected:

Assume the following test zip file:

1/
1/a
1/b
1/c
1/1/
1/1/loop
1/2/
1/2/a
1/2/b
1/2/c
1/3/
2/
2/f
3/
3/g
.dir/
.dir/a/
.dir/a/1
.dir/.1
.dir/.d
.file

where pathnames ending in / are directories and loop is a symlink to ../../1.

With:

p = zipfile.Path("test.zip")

glob() does not allow an empty pattern:

i = p.glob("")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3.13/zipfile/_path/__init__.py", line 419, in glob
    raise ValueError(f"Unacceptable pattern: {pattern!r}")
ValueError: Unacceptable pattern: ''

But rglob() does:

>>> for i in  p.rglob(""):
...     print(i)
test.zip/1/
test.zip/1/1/
test.zip/1/2/
test.zip/1/3/
test.zip/2/
test.zip/3/
test.zip/.dir/
test.zip/.dir/a/

in which case it seems to list only directories.

If using * as pattern:

>>> for i in  p.rglob("*"):
...     print(i)
test.zip/1/a
test.zip/1/b
test.zip/1/c
test.zip/1/1/
test.zip/1/1/loop
test.zip/1/2/
test.zip/1/2/a
test.zip/1/2/b
test.zip/1/2/c
test.zip/1/3/
test.zip/2/f
test.zip/3/g
test.zip/.dir/a/
test.zip/.dir/a/1
test.zip/.dir/.1
test.zip/.dir/.d

the results are also a bit... unexpected...

• . files are included (which is at least not compatible with POSIX pattern matching notation on pathnames)
• some but not all directories are given, e.g. we have 1/1/ but not 1/, which we did get when the pattern is the empty string.
• documentation should also note, that the zip file name is for some reason prepended.

It's also not really clear whether these two functions are safe at all,... e.g. could they be used to break out of traversing the ZIP's contents with some tricky .. (cause at some point the docs warn about Path not doing any such sanitisations).

Cheers,
Chris.

[calestyo]

I've just looked a bit into the code, and it seems there's a special pattern **.

Now:

>>> for i in  p.rglob("**"):
...     print(i)
test.zip/1/
test.zip/1/a
test.zip/1/b
test.zip/1/c
test.zip/1/1/
test.zip/1/1/loop
test.zip/1/2/
test.zip/1/2/a
test.zip/1/2/b
test.zip/1/2/c
test.zip/1/3/
test.zip/2/
test.zip/2/f
test.zip/3/
test.zip/3/g
test.zip/.dir/
test.zip/.dir/a/
test.zip/.dir/a/1
test.zip/.dir/.1
test.zip/.dir/.d

still doesn't yield all elements (.file is missing), but it seems that:

>>> for i in  p.glob("**"):
...     print(i)
test.zip/1/
test.zip/1/a
test.zip/1/b
test.zip/1/c
test.zip/1/1/
test.zip/1/1/loop
test.zip/1/2/
test.zip/1/2/a
test.zip/1/2/b
test.zip/1/2/c
test.zip/1/3/
test.zip/2/
test.zip/2/f
test.zip/3/
test.zip/3/g
test.zip/.dir/
test.zip/.dir/a/
test.zip/.dir/a/1
test.zip/.dir/.1
test.zip/.dir/.d
test.zip/.file

does (which is however a bit counter-intuitive (given the method is just glob not recursiveglob.

[picnixz]

I think it's not meant to be used as such since it's likely inherited from pathlibm.Path. So I would only assume that zipfile.Path interface is the one being documented. cc @jaraco

[calestyo]

I think it's not meant to be used as such since it's likely inherited from pathlibm

But isn't that kind of one of the core ideas of inheritance, that if class C is a child of parent P, that it can be used as such?!

And even if not respectively in special circumstance, shouldn't it then rather define it's own _glob and _rglob and e.g. raise an exception if the "public" ones are used?

[picnixz]

Yes, but the class is documented as only implementing a subset of the methods of Path so one could say that those are undocumented features and that the inheritance is an implementation detail.

[calestyo]

I guess in that case my request would be to make these two part of the public API.

IMO especially using simply .glob("**") to really just recursively iterate over all files is quite nice (and much easier to do than via iterdir().

[picnixz]

cc @barneygale to confirm whether it's a FR or a legitimate bug (I'd say it's more a FR but I may be wrong!)

[barneygale]

glob() does not allow an empty pattern [...] but rglob() does

This matches pathlib.Path behaviour, FWIW. Strictly speaking there's no reason pathlib.Path.glob() should disallow an empty pattern, but it's been that way for ages.

in which case it seems to list only directories.

That's expected IMO - rglob(pattern) is equivalent to glob(join('**', pattern)), and so rglob('') is effectively glob('**/')

If using * as pattern [...] the results are also a bit... unexpected...

• . files are included (which is at least not compatible with POSIX pattern matching notation on pathnames)

Again this matches pathlib behaviour, though pathlib differs from the glob module. See https://docs.python.org/3/library/pathlib.html#comparison-to-the-glob-module

• some but not all directories are given, e.g. we have 1/1/ but not 1/, which we did get when the pattern is the empty string.

Indeed this seems to be a bug.

• documentation should also note, that the zip file name is for some reason prepended.

Personally I'd prefer we stop prepending the zip file name - see jaraco/zipp#134

I think it's not meant to be used as such since it's likely inherited from pathlibm.Path.

Currently zipfile.Path and pathlib.Path share no code. I'm hoping to change that here: jaraco/zipp#132

[calestyo]

cc @barneygale to confirm whether it's a FR or a legitimate bug (I'd say it's more a FR but I may be wrong!)

Maybe it's both ;-) ... perhaps a bit nitpicking, but saying something implements only a subset of something seems to rather mean that e.g. one could get a NotImplemented for some method.
But it does implement glob() and rglob(), they "even" work, but they're not "marked private" like in (_glob() or so).

[calestyo]

That's expected IMO - rglob(pattern) is equivalent to glob(join('', pattern)), and so rglob('') is effectively glob('/')

I've seen that later in the code, too. At least it seems a bit unexpected by the name alone. So if this methods were to became official API, then that peculiarity should perhaps be mentioned in the docs.

Personally I'd prefer we stop prepending the zip file name - see jaraco/zipp#134

I should add, that the .at property has the "correct" path (relative to the ZIP file). And changing this might perhaps break things?

Currently zipfile.Path and pathlib.Path share no code. I'm hoping to change that here: jaraco/zipp#132

Has care been taken to not break existing usages of the current code? I mean I didn't find glob("**") by reading the code, but by examples on the internet using that as a simpler way to iterate over (assumedly) everything in the ZIP.

[thatch]

Talked to @jaraco and we probably need to wait for jaraco/zipp#132 and jaraco/zipp#102 before knowing what needs to be sync'd here.