bpo-28624: Add test for subprocess to check that cmd arg supports PathLike by sayanchowdhury · Pull Request #157 · python/cpython

sayanchowdhury · 2017-02-18T11:26:23Z

No description provided.

the-knights-who-say-ni · 2017-02-18T11:26:24Z

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA. This is necessary for legal reasons before we can look at your contribution. Please follow these steps to help rectify the issue:

If you don't have an account on b.p.o, please create one
Make sure your GitHub username is listed in "Your Details" at b.p.o
If you have not already done so, please sign the PSF contributor agreement
If you just signed the CLA, please wait at least one US business day and then check "Your Details" on bugs.python.org to see if your account has been marked as having signed the CLA (the delay is due to a person having to manually check your signed CLA)
Reply here saying you have completed the above steps

Thanks again to your contribution and we look forward to looking at it!

JelleZijlstra · 2017-02-18T18:27:14Z

make suspicious fails here with "default role used". Not sure, but you may have to put double backticks around os.PathLike.

:ref:os.PathLike would be a preferable fix here (turning this into a cross reference to the ABC's docs)

Actually, if you follow Berker's suggestion below (which seems like a good idea to me), that will also fix this.

berkerpeksag · 2017-02-18T23:19:02Z

Add two spaces before "*cwd* also accepts a os.PathLike object."

It would be better to follow the style in the rest of stdlib:
[...] child. *cmd* can be a :class:`str` and :term:`path-like <path-like object>` object.

And please add a versionchanged directive to properly document os.PathLike support:
.. versionchanged:: 3.6 *cwd* parameter accepts a :term:`path-like object`.

berkerpeksag · 2017-02-18T23:24:01Z

No need to add a comment here.

berkerpeksag · 2017-02-18T23:24:53Z

You can create a pathlib.Path instance from temp_dir and pass it to cwd.

@sayanchowdhury and I were discussing that at the PyCon Pune sprints and I suggested doing it this way to avoid adding a new dependency to the test cases.

sayanchowdhury · 2017-02-19T05:54:21Z

Thanks for the review. I have updated the PR.

ncoghlan · 2017-02-19T10:22:56Z

make patchcheck should clean up the trailing whitespace errors being picked up by Travis.

(Oops, after chatting to @sayanchowdhury, I realised this suggestion wasn't as useful as I thought - make patchcheck only checks uncommitted changes, so it will only help here if you first soft-reset the git state to an earlier commit)

sayanchowdhury · 2017-02-20T14:43:22Z

I ran make patchcheck which removed the trailing spaces. Travis also seems to be happy now.

berkerpeksag

I was going to merge this, but just noticed a small typo. I think this also deserves a Misc/NEWS entry since we are aim to support path-like objects in the stdlib in Python 3.6.

berkerpeksag · 2017-02-22T05:51:45Z

Sorry for the typo. Fixed it.

sayanchowdhury · 2017-02-22T07:25:09Z

Should add to section What's New in Python 3.6.1 release candidate 1 under Library in Misc/NEWS?

zhangyangyu · 2017-02-22T07:35:11Z

Hmm, you only mention string and PathLike, how about bytes? Is it acceptable?

I added a test for bytes too and it's seems to be passing too. Also updated the docs.

berkerpeksag · 2017-02-26T11:32:52Z

Can we please keep this PR specific to os.PathLike support? Passing cwd=b"something" is out of scope for this issue and it should be discussed on bugs.python.org.

Signed-off-by: Sayan Chowdhury <sayan.chowdhury2012@gmail.com>

sayanchowdhury · 2017-02-26T11:40:34Z

Okay, I'll then go ahead and keep the changes os.PathLike specific.

sayanchowdhury · 2017-02-26T13:30:02Z

I have updated the code.

…ts PathLike objects (python#157) (cherry picked from commit d5c11f7)

berkerpeksag · 2017-02-26T17:11:33Z

Thanks!

…ts PathLike objects (#157) (#323) (cherry picked from commit d5c11f7)

#157 added the test, but it's currently (correctly) broken on windows.

ncoghlan · 2017-05-28T05:07:55Z

Added the sprint label, as this PR was submitted at the PyCon Pune 2017 core development sprint.

Detected by gcc -Wmisleading-indentation.

the-knights-who-say-ni added the CLA not signed label Feb 18, 2017

ncoghlan removed the CLA not signed label Feb 18, 2017

the-knights-who-say-ni added the CLA signed label Feb 18, 2017

serhiy-storchaka added type-feature A feature request or enhancement tests Tests in the Lib/test dir and removed type-feature A feature request or enhancement labels Feb 18, 2017

JelleZijlstra reviewed Feb 18, 2017
View reviewed changes

berkerpeksag requested changes Feb 18, 2017
View reviewed changes

berkerpeksag added the needs backport to 3.6 label Feb 18, 2017

sayanchowdhury force-pushed the bpo28624 branch 3 times, most recently from 271a1ad to 1cb8660Compare February 19, 2017 05:53

sayanchowdhury force-pushed the bpo28624 branch from 1cb8660 to da4f28aCompare February 19, 2017 05:55

sayanchowdhury force-pushed the bpo28624 branch from da4f28a to e153659Compare February 19, 2017 10:43

berkerpeksag approved these changes Feb 22, 2017
View reviewed changes

berkerpeksag requested changes Feb 22, 2017
View reviewed changes

sayanchowdhury force-pushed the bpo28624 branch from e153659 to 6c208d2Compare February 22, 2017 07:15

zhangyangyu reviewed Feb 22, 2017
View reviewed changes

sayanchowdhury force-pushed the bpo28624 branch from 6c208d2 to c096044Compare February 26, 2017 11:23

bpo-28624: Add test for Popen to check that cmd arg supports PathLike
3370e0b
Signed-off-by: Sayan Chowdhury <sayan.chowdhury2012@gmail.com>

sayanchowdhury force-pushed the bpo28624 branch from c096044 to 3370e0bCompare February 26, 2017 11:41

Move NEWS entry
0b2c42b

Merge branch 'master' into bpo28624
bfd7de3

berkerpeksag approved these changes Feb 26, 2017
View reviewed changes

berkerpeksag merged commit d5c11f7 into python:masterFeb 26, 2017

berkerpeksag pushed a commit to berkerpeksag/cpython that referenced this pull request Feb 26, 2017
bpo-28624: Add a test that checks that cwd parameter of Popen() accep…
a3e870d
…ts PathLike objects (python#157) (cherry picked from commit d5c11f7)

berkerpeksag removed the needs backport to 3.6 label Feb 26, 2017

berkerpeksag added a commit that referenced this pull request Feb 26, 2017
bpo-28624: Add a test that checks that cwd parameter of Popen() accep…
6b81003
…ts PathLike objects (#157) (#323) (cherry picked from commit d5c11f7)

benjaminp added a commit that referenced this pull request Mar 2, 2017
allow path-like objects to be cwd on windows
e431c29
#157 added the test, but it's currently (correctly) broken on windows.

benjaminp mentioned this pull request Mar 2, 2017
allow path-like objects to be cwd on windows #389
Merged

benjaminp added a commit that referenced this pull request Mar 2, 2017
allow path-like objects to be cwd on windows (#389)
f522792
#157 added the test, but it's currently (correctly) broken on windows.

ncoghlan added the sprint label May 28, 2017

akruis added a commit to akruis/cpython that referenced this pull request Jun 20, 2018
Stackless issuepython#157: a trivial indentation fix
d119676
Detected by gcc -Wmisleading-indentation.

jaraco pushed a commit that referenced this pull request Dec 2, 2022
Print rate limit remaining (#157)
1138fd5

Uh oh!

Uh oh!

Conversation

sayanchowdhury commented Feb 18, 2017

Uh oh!

the-knights-who-say-ni commented Feb 18, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sayanchowdhury commented Feb 19, 2017

Uh oh!

ncoghlan commented Feb 19, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sayanchowdhury commented Feb 20, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

berkerpeksag left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sayanchowdhury commented Feb 22, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

berkerpeksag commented Feb 26, 2017

Uh oh!

sayanchowdhury commented Feb 26, 2017

Uh oh!

sayanchowdhury commented Feb 26, 2017

Uh oh!

berkerpeksag commented Feb 26, 2017

Uh oh!

ncoghlan commented May 28, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

ncoghlan commented Feb 19, 2017•
edited
Loading

sayanchowdhury commented Feb 20, 2017•
edited
Loading