shutil.copytree - 3.8 changed argument types of the ignore callback

[mbarkhau]

BPO	39390
Nosy	@vstinner, @giampaolo, @serhiy-storchaka, @mbarkhau
PRs	bpo-39390: Update shutil.copytree doc - Add versionchanged 3.8 notice for ignore callable #18069 bpo-39390: fix argument types for ignore callback of shutil.copytree #18122 [3.8] bpo-39390 shutil: fix argument types for ignore callback (GH-18122). #18168

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/giampaolo'
closed_at = <Date 2020-01-27.23:48:24.425>
created_at = <Date 2020-01-19.20:40:46.651>
labels = ['3.8', 'type-bug', 'library', '3.9', 'docs']
title = 'shutil.copytree - 3.8 changed argument types of the ignore callback'
updated_at = <Date 2020-01-27.23:48:24.425>
user = 'https://github.com/mbarkhau'

bugs.python.org fields:

activity = <Date 2020-01-27.23:48:24.425>
actor = 'giampaolo.rodola'
assignee = 'giampaolo.rodola'
closed = True
closed_date = <Date 2020-01-27.23:48:24.425>
closer = 'giampaolo.rodola'
components = ['Documentation', 'Library (Lib)']
creation = <Date 2020-01-19.20:40:46.651>
creator = 'mbarkhau'
dependencies = []
files = []
hgrepos = []
issue_num = 39390
keywords = ['patch', '3.8regression']
message_count = 14.0
messages = ['360271', '360300', '360303', '360427', '360431', '360435', '360437', '360621', '360622', '360624', '360625', '360626', '360627', '360825']
nosy_count = 5.0
nosy_names = ['vstinner', 'giampaolo.rodola', 'docs@python', 'serhiy.storchaka', 'mbarkhau']
pr_nums = ['18069', '18122', '18168']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue39390'
versions = ['Python 3.8', 'Python 3.9']

[mbarkhau]

In Python 3.8, the types of the parameters to the ignore callable appear to have changed.

Previously the src parameter was a string and the names parameter was a list of strings. Now the src parameter appears to be either a pathlib.Path or an os.DirEntry, while the names parameter is a set of strings.

I would suggest adding the following to the documentation https://github.com/python/cpython/blob/master/Doc/library/shutil.rst

.. versionchanged:: 3.8
The types of arguments to *ignore* have changed. The first argument
(the directory being visited) is a func:`os.DirEntry` or a
func:`pathlib.Path`; Previously it was a string. The second argument is
a set of strings; previously it was a list of strings.

[serhiy-storchaka]

This looks like a backward incompatible change in 3.8. Should not copytree convert arguments of the ignore callback to str and list correspondingly?

[mbarkhau]

This looks like a backward incompatible change in 3.8.

Indeed.

Should not copytree convert arguments of the ignore callback to str and list correspondingly?

Well, since any existing code probably expects that behavior (or at least probably works if that is the case), I would be for such a change. I'm not sure what your policy is though. If there is recently written code that assumes the new types, are you OK with that breaking if it is changed back?

I guess since it's an undocumented breaking change, it shouldn't be too much of an issue.

[mbarkhau]

Is there anything I can do to help move this forward?

[giampaolo]

Should not copytree convert arguments of the ignore callback to str and list correspondingly?

It should. I think it makes sense to just do this for Python 3.8.2 instead of updating the doc.

[mbarkhau]

Unless somebody else wants to, I could have a go at an PR to update shutil.py

[giampaolo]

Yes, thanks. Whoever got bit by this is either getting an exception or not the intended behavior (due to failed string comparison). I doubt anybody is relying on the new type checking since it's not documented. If they are, they are probably just doing:

    def callback(name, names):
        if not isinstance(name, str):  # bugfix 3.8
            name = name.name
        ...

[giampaolo]

New changeset 8870433 by Giampaolo Rodola (mbarkhau) in branch 'master':
bpo-39390 shutil: fix argument types for ignore callback (GH-18122)
8870433

[giampaolo]

For completeness, a similar problem is present also on python < 3.8 if passing a pathlib.Path type as *src*: the callback function will receive a pathlib.Path type once, and then string types.

[mbarkhau]

For completeness, a similar problem is present also on python < 3.8

Fair point. I'll have a look.

[serhiy-storchaka]

For completeness, a similar problem is present also on python < 3.8 if passing a pathlib.Path type as *src*

I do not think this is a problem. If you pass a string, you will get a string, so existing code will continue to work as before. The only problem if you pass a string, but get an unexpected type.

[giampaolo]

I don't think we need to change anything on < 3.8, but 3.8 and 3.9 will always convert *src* to str via os.fspath(), which IMO is more consistent (e.g. os.path.* functions and others do the same).

[mbarkhau]

If you pass a string, you will get a string, so existing code will continue to work as before.

Somebody might have code that is running against a flat directory and have written their ignore function expecting to get a pathlib.Path, because that's the only case they encountered. This change would break their code and so would an upgrade to 3.9 with the patch that was just merged.

[giampaolo]

New changeset cf9d005 by Giampaolo Rodola (mbarkhau) in branch '3.8':
[3.8] bpo-39390 shutil: fix argument types for ignore callback (GH-18122)
cf9d005