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

compatibility issue with python 3.6 #16

Open
jaccol opened this issue May 9, 2024 · 3 comments
Open

compatibility issue with python 3.6 #16

jaccol opened this issue May 9, 2024 · 3 comments

Comments

@jaccol
Copy link
Contributor

jaccol commented May 9, 2024

Hi,

I notice a last compatibility issue with python 3.6 (and probably earlier)
I get an error TypeError: __init__() got an unexpected keyword argument 'capture_output'

The issue is in lines 193 and 201 in the subprocess.run() function. Both capture_output and text are new additions to the function per python version 3.7. I Believe that stdout=PIPE, stderr=PIPE, universal_newlines=True is identical and is supported in older versions of python.
Obviously PIPE needs to be imported from subprocess.

Jacco

@pbiering
Copy link
Collaborator

pbiering commented May 9, 2024

this was introduced by 30bbd75

I will take care adding next fallback support, keep you updated

@jaccol
Copy link
Contributor Author

jaccol commented May 9, 2024

Hi Peter,

Thanks for the quick PR.
2 things though:

  1. I think (untested) that you miss the stderr=PIPE in the second run command.
  2. I was under the impression, reading through the documentation, that the alternative I gave was identical, but supported by more python versions. I thought that there was no need for the discrimination. (But your code will work)

If capture_output is true, stdout and stderr will be captured. When used, the internal Popen object is automatically created with stdout and stdin both set to PIPE. The stdout and stderr arguments may not be supplied at the same time as capture_output. If you wish to capture and combine both streams into one, set stdout to PIPE and stderr to STDOUT, instead of using capture_output.

and

Changed in version 3.7: Added the text parameter as an alias for universal_newlines.

(https://docs.python.org/3/library/subprocess.html)

Hmm, rereading the docs I might have mixed-up stdin and stderr in the second sentence. After some test, I think that stderr=PIPE prints intelligible errors, while stderr=STDOUT does not.

Jacco

@pbiering
Copy link
Collaborator

1. I think (untested) that you miss the `stderr=PIPE` in the second `run` command.

Overseen, fixed by 9e471cc

2. I was under the impression, reading through the documentation, that the alternative I gave was identical, but supported by more python versions. I thought that there was no need for the discrimination. (But your code will work)

I have zero trust in Python related to long term API stability in their functions, that's why I'm using Python version dependent coding...who knows when one decide to discard the support of legacy options in a future Python version...with current coding this would not harm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants