bpo-29901: Improve support of path-like objects in zipapp.#815
Conversation
Now general path-like objects are supported, not just pathlib.Path.
|
@serhiy-storchaka, thanks for your PR! By analyzing the history of the files in this pull request, we identified @brettcannon, @pfmoore and @terryjreedy to be potential reviewers. |
pfmoore
left a comment
pfmoore
left a comment
There was a problem hiding this comment.
Looks good to me - I made one comment noting a couple of points, but I don't believe they are issues.
| z.write(str(child), arcname) | ||
| for child in source.rglob('*'): | ||
| arcname = child.relative_to(source).as_posix() | ||
| z.write(child, arcname) |
There was a problem hiding this comment.
I assume that ZipFile.write() now takes a general path object, and doesn't require conversion to a string? The docs for that function don't explicitly state this, unlike some other functions in that module, but I assume that's just a documentation oversight.
Also, there's a minor change in behaviour here, as the archive name is always in POSIX format now, whereas before it was in OS-specific format. But IMO that's entirely reasonable (and was an oversight on my part in the original code) so I'm happy with that.
There was a problem hiding this comment.
There is no behavior change. ZipFile itself converts the archive name to POSIX format. But I think it is better to do this explicitly. Maybe implicit conversion will be deprecated in future.
There was a problem hiding this comment.
Ah, OK. I agree, explicit is better here.
4 participants
Now general path-like objects are supported, not just pathlib.Path.