[issue40358] pathlib's relative_to should behave like os.path.relpath
Domenico Ragusa added the comment: I've solved the conflicts with GH-19611 (bpo-23082: Better error message for PurePath.relative_to() from pathlib) that was merged in the mean time and improved the documentation. Everything appears to be in order, can you take a look at it? -- versions: +Python 3.10 -Python 3.9 ___ Python tracker <https://bugs.python.org/issue40358> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue40506] add support for os.Pathlike filenames in zipfile.ZipFile.writestr
Change by Domenico Ragusa : -- pull_requests: +19314 stage: -> patch review pull_request: https://github.com/python/cpython/pull/20002 ___ Python tracker <https://bugs.python.org/issue40506> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue40506] add support for os.Pathlike filenames in zipfile.ZipFile.writestr
Domenico Ragusa added the comment: Here's a small patch to do this. Everything seems to work fine. I don't know if where I placed the test (in OtherTests) is the most appropriate. On Tue, May 5, 2020 at 4:12 AM Domenico Ragusa wrote: > > > New submission from Domenico Ragusa : > > ZipFile seems to support Pathlike objects pretty well, except in > ZipFile.writestr. > For example: > > >>> a = ZipFile(Path('test.zip'), 'w') # this works ok > >>> a.write(Path('./foo.jpeg'), arcname=PurePath('/some/thing.jpeg')) # this > >>> works as well > >>> a.writestr(PurePath('/test.txt'), 'idk') # this doesn't > Traceback (most recent call last): > File "", line 1, in > File "/usr/lib/python3.8/zipfile.py", line 1788, in writestr > zinfo = ZipInfo(filename=zinfo_or_arcname, > File "/usr/lib/python3.8/zipfile.py", line 349, in __init__ > null_byte = filename.find(chr(0)) > AttributeError: 'PurePosixPath' object has no attribute 'find' > > I think it would be more consistent if it accepted any kind of paths, it > would suffice to call os.fspath in ZipInfo.__init__ when the filename is a > Pathlike-object, it's just 2 lines (+ tests, of course). > > Can I go ahead and prepare a patch for this? > > -- > components: Library (Lib) > messages: 368098 > nosy: d.ragusa > priority: normal > severity: normal > status: open > title: add support for os.Pathlike filenames in zipfile.ZipFile.writestr > type: enhancement > versions: Python 3.9 > > ___ > Python tracker > <https://bugs.python.org/issue40506> > ___ -- keywords: +patch Added file: https://bugs.python.org/file49132/pathlike_writestr.patch ___ Python tracker <https://bugs.python.org/issue40506> ___diff --git a/Lib/test/test_zipfile.py b/Lib/test/test_zipfile.py index 29d98c8092..31c83987ab 100644 --- a/Lib/test/test_zipfile.py +++ b/Lib/test/test_zipfile.py @@ -1546,6 +1546,12 @@ class OtherTests(unittest.TestCase): zinfo.flag_bits |= 0x08 # Include an extended local header. orig_zip.writestr(zinfo, data) +def test_writestr_pathlike_issue40506(self): +with zipfile.ZipFile(TESTFN2, 'w') as orig_zip: +path = '/foo/bar.txt' +orig_zip.writestr(pathlib.PurePath(path), '1234') +self.assertEqual(orig_zip.open(path).read(4), b'1234') + def test_close(self): """Check that the zipfile is closed after the 'with' block.""" with zipfile.ZipFile(TESTFN2, "w") as zipfp: diff --git a/Lib/zipfile.py b/Lib/zipfile.py index 8903d6a42e..44b3ee8e63 100644 --- a/Lib/zipfile.py +++ b/Lib/zipfile.py @@ -341,6 +341,8 @@ class ZipInfo (object): ) def __init__(self, filename="NoName", date_time=(1980,1,1,0,0,0)): +if isinstance(filename, os.PathLike): +filename = os.fspath(filename) self.orig_filename = filename # Original file name in archive # Terminate the file name at the first null byte. Null bytes in file ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue40506] add support for os.Pathlike filenames in zipfile.ZipFile.writestr
New submission from Domenico Ragusa : ZipFile seems to support Pathlike objects pretty well, except in ZipFile.writestr. For example: >>> a = ZipFile(Path('test.zip'), 'w') # this works ok >>> a.write(Path('./foo.jpeg'), arcname=PurePath('/some/thing.jpeg')) # this >>> works as well >>> a.writestr(PurePath('/test.txt'), 'idk') # this doesn't Traceback (most recent call last): File "", line 1, in File "/usr/lib/python3.8/zipfile.py", line 1788, in writestr zinfo = ZipInfo(filename=zinfo_or_arcname, File "/usr/lib/python3.8/zipfile.py", line 349, in __init__ null_byte = filename.find(chr(0)) AttributeError: 'PurePosixPath' object has no attribute 'find' I think it would be more consistent if it accepted any kind of paths, it would suffice to call os.fspath in ZipInfo.__init__ when the filename is a Pathlike-object, it's just 2 lines (+ tests, of course). Can I go ahead and prepare a patch for this? -- components: Library (Lib) messages: 368098 nosy: d.ragusa priority: normal severity: normal status: open title: add support for os.Pathlike filenames in zipfile.ZipFile.writestr type: enhancement versions: Python 3.9 ___ Python tracker <https://bugs.python.org/issue40506> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue40358] pathlib's relative_to should behave like os.path.relpath
Change by Domenico Ragusa : -- pull_requests: +19133 pull_request: https://github.com/python/cpython/pull/19813 ___ Python tracker <https://bugs.python.org/issue40358> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue40358] pathlib's relative_to should behave like os.path.relpath
Domenico Ragusa added the comment: I may have forgotten to use the proper format for the title of each commit, should I delete the pull request and make a new one or can it be fixed when (or if) it's pulled? On Thu, Apr 30, 2020 at 2:03 AM Roundup Robot wrote: > > > Change by Roundup Robot : > > > -- > nosy: +python-dev > nosy_count: 5.0 -> 6.0 > pull_requests: +19128 > stage: -> patch review > pull_request: https://github.com/python/cpython/pull/19807 > > ___ > Python tracker > <https://bugs.python.org/issue40358> > ___ -- ___ Python tracker <https://bugs.python.org/issue40358> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue40358] pathlib's relative_to should behave like os.path.relpath
Domenico Ragusa added the comment: Yeah, you're right, there's no access to the filesystem and the result is generated assuming the paths are already resolved. `strict` seems to be an appropriate name for the option, thanks. I've looked into the test suite, it helped a lot especially with Windows paths, they were more complicated than I though. I've duplicated the tests to verify that it still function as before and I've added some tests for values that would raise an exception. It works. I'm not overly fond of the way I check for unrelated paths, but I couldn't think of something more elegant. Any input is appreciated. -- Added file: https://bugs.python.org/file49095/relative_to.patch ___ Python tracker <https://bugs.python.org/issue40358> ___diff --git a/Lib/pathlib.py b/Lib/pathlib.py index f98d69e..eb25761 100644 --- a/Lib/pathlib.py +++ b/Lib/pathlib.py @@ -895,10 +895,10 @@ class PurePath(object): return self._from_parsed_parts(self._drv, self._root, self._parts[:-1] + [name]) -def relative_to(self, *other): +def relative_to(self, *other, strict=True): """Return the relative path to another path identified by the passed arguments. If the operation is not possible (because this is not -a subpath of the other path), raise ValueError. +related to the other path), raise ValueError. """ # For the purpose of this method, drive and root are considered # separate parts, i.e.: @@ -918,14 +918,31 @@ class PurePath(object): to_abs_parts = [to_drv, to_root] + to_parts[1:] else: to_abs_parts = to_parts + n = len(to_abs_parts) cf = self._flavour.casefold_parts -if (root or drv) if n == 0 else cf(abs_parts[:n]) != cf(to_abs_parts): +common = 0 +for p, tp in zip(cf(abs_parts), cf(to_abs_parts)): +if p != tp: +break +common += 1 + +if strict: +failure = (root or drv) if n == 0 else common != n +error_message = "{!r} does not start with {!r}" +up_parts = [] +else: +failure = root != to_root +if drv or to_drv: +failure = cf([drv]) != cf([to_drv]) or (failure and n > 1) +error_message = "{!r} is not related to {!r}" +up_parts = (n-common)*['..'] + +if failure: formatted = self._format_parsed_parts(to_drv, to_root, to_parts) -raise ValueError("{!r} does not start with {!r}" - .format(str(self), str(formatted))) -return self._from_parsed_parts('', root if n == 1 else '', - abs_parts[n:]) +raise ValueError(error_message.format(str(self), str(formatted))) +return self._from_parsed_parts('', root if common == 1 else '', + up_parts+abs_parts[common:]) def is_relative_to(self, *other): """Return True if the path is relative to another path or False. diff --git a/Lib/test/test_pathlib.py b/Lib/test/test_pathlib.py index 1589282..a6d8fe4 100644 --- a/Lib/test/test_pathlib.py +++ b/Lib/test/test_pathlib.py @@ -613,13 +613,30 @@ class _BasePurePathTest(object): self.assertEqual(p.relative_to('a/'), P('b')) self.assertEqual(p.relative_to(P('a/b')), P()) self.assertEqual(p.relative_to('a/b'), P()) +self.assertEqual(p.relative_to(P(), strict=False), P('a/b')) +self.assertEqual(p.relative_to('', strict=False), P('a/b')) +self.assertEqual(p.relative_to(P('a'), strict=False), P('b')) +self.assertEqual(p.relative_to('a', strict=False), P('b')) +self.assertEqual(p.relative_to('a/', strict=False), P('b')) +self.assertEqual(p.relative_to(P('a/b'), strict=False), P()) +self.assertEqual(p.relative_to('a/b', strict=False), P()) +self.assertEqual(p.relative_to(P('a/c'), strict=False), P('../b')) +self.assertEqual(p.relative_to('a/c', strict=False), P('../b')) +self.assertEqual(p.relative_to(P('a/b/c'), strict=False), P('..')) +self.assertEqual(p.relative_to('a/b/c', strict=False), P('..')) +self.assertEqual(p.relative_to(P('c'), strict=False), P('../a/b')) +self.assertEqual(p.relative_to('c', strict=False), P('../a/b')) # With several args. self.assertEqual(p.relative_to('a', 'b'), P()) +self.assertEqual(p.relative_to('a', 'b', strict=False), P()) # Unrelated paths. self.assertRaises(ValueError, p.relative_to, P('c')) self.assertRaises(ValueError, p.relative_to, P('a/b/c')) self.assertRaises(ValueError, p.relative_to, P('a/c'))
[issue40358] pathlib's relative_to should behave like os.path.relpath
Change by Domenico Ragusa : Removed file: https://bugs.python.org/file49081/pathlib.diff ___ Python tracker <https://bugs.python.org/issue40358> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue40358] pathlib's relative_to should behave like os.path.relpath
Domenico Ragusa added the comment: Thanks for your answer. Yeah, I'm new, I'm reading the guide, sorry for any faux pas :) Ok, an option would be great as well, a simple True/False switch? Any suggestion for the name? I'll get back with a proper patch this time. On Wed, Apr 22, 2020 at 8:18 PM Antoine Pitrou wrote: > > Antoine Pitrou added the comment: > > The current behaviour is by design. I would not mind adding an option to > control it, though. > > If you are new to Python development and want to submit a patch or PR, I > recommend reading the Developer's Guide: > https://devguide.python.org/ > > -- > > ___ > Python tracker > <https://bugs.python.org/issue40358> > ___ > -- ___ Python tracker <https://bugs.python.org/issue40358> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue40358] pathlib's relative_to should behave like os.path.relpath
New submission from Domenico Ragusa : Can we improve pathlib.relative_to(other) so that it handles the case of a path not being a direct child of other, like os.path.relpath? For example: Path('/some/thing').relative_to('/foo') -> Path('../some/thing') At the moment it just raises an exception. -- components: Library (Lib) files: pathlib.diff keywords: patch messages: 366958 nosy: Domenico Ragusa priority: normal severity: normal status: open title: pathlib's relative_to should behave like os.path.relpath type: enhancement versions: Python 3.9 Added file: https://bugs.python.org/file49081/pathlib.diff ___ Python tracker <https://bugs.python.org/issue40358> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com