Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unclear documentation for shutil.copytree() #88513

Closed
tilmanvogel mannequin opened this issue Jun 8, 2021 · 5 comments
Closed

Unclear documentation for shutil.copytree() #88513

tilmanvogel mannequin opened this issue Jun 8, 2021 · 5 comments
Assignees
Labels
3.9 only security fixes 3.10 only security fixes 3.11 bug and security fixes docs Documentation in the Doc dir stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@tilmanvogel
Copy link
Mannequin

tilmanvogel mannequin commented Jun 8, 2021

BPO 44347
Nosy @iritkatriel, @jdevries3133
PRs
  • bpo-44347: [doc] clarify meaning of shutil.copytree's dirs_exist_ok kwarg #26634
  • bpo-44347: [doc] clarify meaning of shutil.copytree's dirs_exist_ok kwarg #26643
  • 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 = None
    closed_at = None
    created_at = <Date 2021-06-08.09:17:12.166>
    labels = ['3.9', '3.10', '3.11', 'type-feature', 'library', 'docs']
    title = 'Unclear documentation for shutil.copytree()'
    updated_at = <Date 2021-06-10.12:29:33.310>
    user = 'https://bugs.python.org/tilmanvogel'

    bugs.python.org fields:

    activity = <Date 2021-06-10.12:29:33.310>
    actor = 'jack__d'
    assignee = 'docs@python'
    closed = False
    closed_date = None
    closer = None
    components = ['Documentation', 'Library (Lib)']
    creation = <Date 2021-06-08.09:17:12.166>
    creator = 'tilman.vogel'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 44347
    keywords = ['patch']
    message_count = 4.0
    messages = ['395315', '395452', '395489', '395494']
    nosy_count = 4.0
    nosy_names = ['docs@python', 'iritkatriel', 'tilman.vogel', 'jack__d']
    pr_nums = ['26634', '26643']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue44347'
    versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']

    @tilmanvogel
    Copy link
    Mannequin Author

    tilmanvogel mannequin commented Jun 8, 2021

    I donot understand this sentence:

    "dirs_exist_ok dictates whether to raise an exception in case dst or any
    missing parent directory already exists."

    How can a "missing parent directory already exist"?

    My understanding would be that an existing dst would be OK (and copied into) but missing parent directories are just the ones above dst that also don't exist.

    Until 3.7, missing parent directories were documented to be auto-created (mkdir -p-style) according to the documentation ("The destination directory, named by dst, must not already exist; it will be created as well as missing parent directories."). Was this feature really removed? If not, then this part was accidentally (?) dropped from documentation?

    What am I missing? I think, the documentation should be amended to make that clear.

    @tilmanvogel tilmanvogel mannequin added 3.8 only security fixes 3.10 only security fixes 3.9 only security fixes labels Jun 8, 2021
    @tilmanvogel tilmanvogel mannequin assigned docspython Jun 8, 2021
    @tilmanvogel tilmanvogel mannequin added docs Documentation in the Doc dir type-feature A feature request or enhancement 3.8 only security fixes 3.10 only security fixes 3.9 only security fixes labels Jun 8, 2021
    @tilmanvogel tilmanvogel mannequin assigned docspython Jun 8, 2021
    @tilmanvogel tilmanvogel mannequin added docs Documentation in the Doc dir type-feature A feature request or enhancement labels Jun 8, 2021
    @iritkatriel
    Copy link
    Member

    From the code, this arg is simply passed to os.mkdirs() here:

    os.makedirs(dst, exist_ok=dirs_exist_ok)

    os.mkdir is documented here: https://docs.python.org/3/library/os.html#os.makedirs

    and says just "If exist_ok is False (the default), an FileExistsError is raised if the target directory already exists."

    So I agree that the sentence you highlighted can be improved.

    Would you like to submit a patch?

    @iritkatriel iritkatriel added stdlib Python modules in the Lib dir 3.11 bug and security fixes and removed 3.8 only security fixes labels Jun 9, 2021
    @jdevries3133
    Copy link
    Mannequin

    jdevries3133 mannequin commented Jun 9, 2021

    I would like to submit a patch for this. This would be my first contribution :)

    I am starting on a patch now.

    @jdevries3133
    Copy link
    Mannequin

    jdevries3133 mannequin commented Jun 9, 2021

    I've created a PR: #26634

    I forgot to put a news entry, but hopefully that's ok since this is a very small change.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    JelleZijlstra pushed a commit that referenced this issue Apr 12, 2022
    * add a paragraph to document this kwarg in detail
    * update docstring in the source accordingly
    miss-islington pushed a commit to miss-islington/cpython that referenced this issue Apr 12, 2022
    …-91434)
    
    * add a paragraph to document this kwarg in detail
    * update docstring in the source accordingly
    (cherry picked from commit f33e2c8)
    
    Co-authored-by: Jack DeVries <jdevries3133@gmail.com>
    @JelleZijlstra JelleZijlstra self-assigned this Apr 12, 2022
    @JelleZijlstra
    Copy link
    Member

    JelleZijlstra commented Apr 12, 2022

    #91434 was merged to address this. Thanks for your contribution @jdevries3133! I'll close this issue once the backports (#91464, #91465) are merged.

    JelleZijlstra pushed a commit that referenced this issue Apr 13, 2022
    …91465)
    
    * add a paragraph to document this kwarg in detail
    * update docstring in the source accordingly
    (cherry picked from commit f33e2c8)
    
    Co-authored-by: Jack DeVries <jdevries3133@gmail.com>
    ambv pushed a commit that referenced this issue Apr 15, 2022
    …91464)
    
    * add a paragraph to document this kwarg in detail
    * update docstring in the source accordingly
    (cherry picked from commit f33e2c8)
    
    Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
    hello-adam pushed a commit to hello-adam/cpython that referenced this issue Jun 2, 2022
    …-91434) (pythonGH-91465)
    
    * add a paragraph to document this kwarg in detail
    * update docstring in the source accordingly
    (cherry picked from commit f33e2c8)
    
    Co-authored-by: Jack DeVries <jdevries3133@gmail.com>
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.9 only security fixes 3.10 only security fixes 3.11 bug and security fixes docs Documentation in the Doc dir stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants