GeezerVison

I don't think mixing str and bytes paths works. The source code has this:

real_dst = os.path.join(dst, _basename(src))

which fails if it tries to join strings and bytes.

I don't think mixing str and bytes paths works. The source code has this:

real_dst = os.path.join(dst, _basename(src))

which fails if it tries to join strings and bytes.

This def works for me:

>>> import shutil
>>> shutil.move("test.txt", b"test2.txt")
b'test2.txt'
>>> shutil.move(b"test2.txt", "test.txt")
'test.txt'
>>> shutil.move("some_folder", "some_folder2")
'some_folder2'
>>> shutil.move(b"some_folder2", b"some_folder")
b'some_folder'
>>> shutil.move(b"some_folder", "some_folder2")
'some_folder2'
>>> shutil.move("some_folder2", b"some_folder")
b'some_folder'

EDIT: I noticed that bit in the source code only impacts directories, so I've added them above. They still seem to work though.

The code I mentioned runs when dst is a directory.

The code I mentioned runs when dst is a directory.

Ahh it's only when dst is an existing directory.

In that case yes I get errors:

>>> shutil.move("some_folder", b"some_existing_folder")
...
TypeError: Can't mix strings and bytes in path components
>>> shutil.move(b"some_folder", "some_exising_folder")
...
File "/usr/local/Cellar/[email protected]/3.9.9/Frameworks/Python.framework/Versions/3.9/lib/python3.9/shutil.py", line 771, in _basename
    return os.path.basename(path.rstrip(sep))
TypeError: a bytes-like object is required, not 'str'

Feels like a bug given that it's only in this section of the code...

The second of those issues seems like a bigger issue since this would also fail:

>>> shutil.move(b"some_folder", b"some_exising_folder")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/Cellar/[email protected]/3.9.9/Frameworks/Python.framework/Versions/3.9/lib/python3.9/shutil.py", line 810, in move
    real_dst = os.path.join(dst, _basename(src))
  File "/usr/local/Cellar/[email protected]/3.9.9/Frameworks/Python.framework/Versions/3.9/lib/python3.9/shutil.py", line 771, in _basename
    return os.path.basename(path.rstrip(sep))
TypeError: a bytes-like object is required, not 'str'

With those test cases, I think we should close this PR since bytes will always break the existing folder case.

Do you happen to know where the best place flag this issue is for cpython?

Edit: disregard my comment, I just pattern matched to a thing I'd seen before (shutil has historically not supported path objects well, in general worth double checking these kinds of changes against older Pythons)

No worries, it looks like the types shutil functions accept is a bit inconsistent in general (although looks like they're improving) 😄

I'm working through the other shutil functions atm as I'm finding a lot of them work fine with bytes. Once I've gone through all the others we should hopefully have quite a strong precedent for getting move updated to accept bytes arguments in the existing dst case.

In the meantime, maybe it's worth me removing my changes and just adding a comment above the move stub mentioning the reasoning for not accepting bytes argument (or a link to this PR) so people are aware of our reasoning for not changing it (given that from my examples its easy to be misled by this function)?

In the meantime, maybe it's worth me removing my changes and just adding a comment above the move stub mentioning the reasoning for not accepting bytes argument (or a link to this PR) so people are aware of our reasoning for not changing it (given that from my examples its easy to be misled by this function)?

That seems right to me.

I've reverted the typing changes and added a comment linking to this PR to highlight for future contributors 👍

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

Do you happen to know where the best place flag this issue is for cpython?

bugs.python.org. My experience with reporting and fixing bugs in cpython hasn't been great, but you might have better luck.