Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

CLA Missing

Our records indicate the following people have not signed the CLA:

@y0urself

For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

@barneygale what do you think?

I'm supportive of adding chown() - it comes up semi-regularly in devops work.

I don't love lchown() - as you say @JelleZijlstra, it's two ways of spelling the same thing.

Couple more things to consider/discuss:

• We already have owner() and group() methods, which return names, not IDs. Would it make sense to accept names here too?
• We don't currently expose dir_fd in pathlib, why start here? :)

I'm fine with adding chown().

I also concur on not wanting lchown().

No opinion on the name/ID question.

I don't quite follow the dir_fd comment as I don't see that in the PR.

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

I don't quite follow the dir_fd comment as I don't see that in the PR.

I have already removed the dir_fd option.

I am not sure with owner/ group stuff. They return the current owner/group for the given Path object, but these parameters are the ones, we want to change with chown.
If we allow (new_)owner here, we would probably need to go with pwd.getpwnam('new_owner').pw_uid.

On the other hand I have removed lchown() from the PR and I have made the requested changes; please review again.

Thanks for making the requested changes!

@brettcannon: please review the changes made to this pull request.

All commit authors signed the Contributor License Agreement.

Having given it more thought, and considering the context where this is likely to be used (devops-y scripts), I think supporting owner and group names (in addition to IDs) is important. I think the new method should call through to shutil.chown(), which already accepts int | str | None for each argument. I think the arguments should be spelled owner and group for symmetry with the Path methods. What do you reckon?

Having given it more thought, and considering the context where this is likely to be used (devops-y scripts), I think supporting owner and group names (in addition to IDs) is important. I think the new method should call through to shutil.chown(), which already accepts int | str | None for each argument. I think the arguments should be spelled owner and group for symmetry with the Path methods. What do you reckon?

We would need to drop the follow_symlinks flag, when we use shutil.chown().

Or add follow_symlinks to shutil.chown()

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

I left a comment on the issue, but after thinking about it, right now there isn't really enough here to warrant taking this PR as it's just wrapping a function call in a method. Without some added benefit to being attached to Path (e.g. higher-level), having to support this API for decades is unfortunately not going to be worth it.

Thanks for the PR, @y0urself. I'm afraid I'm going to close this as rejected, as I agree with Brett's comments, as well as @barneygale's and @CAM-Gerlach's remarks on the associated issue. However, I hope this doesn't discourage you from contributing to CPython in the future! We do appreciate the contribution :)

A few words about one thing.
It will be super cool to have this method because there is awesome python-powered xonsh shell and Path objects are using in xonsh broadly. You can do this in the shell using path-strings:

pip install -U 'xonsh[full]'
xonsh

cd /tmp && echo world > hello.txt 

p'hello.txt'
# Path('hello.txt')
p'hello.txt'.read_text()
# 'world\n'

for f in p'/tmp'.glob('hello*'):
    print(f, f.exists())
# /tmp/hello.txt True

It will be cool to run p'hello.txt'.chown('root') instead of running subprocess chown root hello.txt (it's slower) or do import os etc.

I'm for reopening this issue and favor for enriching pathlib.

@barneygale , as you're now officially the pathlib maintainer, what is your take on considering re-opening this PR and/or a revised one?

I've re-opened the issue, we could continue discussion there if that works?