Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.python > #90876 > unrolled thread

Best way to rewrite Popen

Back to article view | Back to comp.lang.python

Contents

  Best way to rewrite Popen Cecil Westerhof <Cecil@decebal.nl> - 2015-05-19 19:01 +0200
    Re: Best way to rewrite Popen Jon Ribbens <jon+usenet@unequivocal.co.uk> - 2015-05-19 17:36 +0000
      Re: Best way to rewrite Popen Cecil Westerhof <Cecil@decebal.nl> - 2015-05-19 21:13 +0200
        Re: Best way to rewrite Popen Jon Ribbens <jon+usenet@unequivocal.co.uk> - 2015-05-19 20:14 +0000
        Re: Best way to rewrite Popen Cecil Westerhof <Cecil@decebal.nl> - 2015-05-19 22:19 +0200
          Re: Best way to rewrite Popen Jon Ribbens <jon+usenet@unequivocal.co.uk> - 2015-05-19 21:28 +0000
            Re: Best way to rewrite Popen Cecil Westerhof <Cecil@decebal.nl> - 2015-05-20 00:23 +0200
              Re: Best way to rewrite Popen MRAB <python@mrabarnett.plus.com> - 2015-05-20 00:20 +0100
                Re: Best way to rewrite Popen Cecil Westerhof <Cecil@decebal.nl> - 2015-05-20 02:15 +0200
    Re: Best way to rewrite Popen Jonas Wielicki <jonas@wielicki.name> - 2015-05-19 19:43 +0200
    Re: Best way to rewrite Popen Chris Angelico <rosuav@gmail.com> - 2015-05-20 03:47 +1000
    Re: Best way to rewrite Popen Zachary Ware <zachary.ware+pylist@gmail.com> - 2015-05-19 12:55 -0500
    Re: Best way to rewrite Popen Chris Angelico <rosuav@gmail.com> - 2015-05-20 04:13 +1000
    Re: Best way to rewrite Popen Thomas 'PointedEars' Lahn <PointedEars@web.de> - 2015-05-20 03:23 +0200

#90876 — Best way to rewrite Popen

At the moment I am playing with things like:
    p = subprocess.Popen('ls -l', shell = True, stdout = subprocess.PIPE)

I think that most of the times this are the values I want. So it would
be nice to overrule the defaults. What is the best way to do this? So
creating a function that is exactly the same except for the defaults
for shell and stdout (and maybe stderr).


It is a little less important as I first thought, because I found the
following:
    error, output = subprocess.getstatusoutput('ls -1')
    files_new = output.splitlines()
But it is still nice to know.

-- 
Cecil Westerhof
Senior Software Engineer
LinkedIn: http://www.linkedin.com/in/cecilwesterhof

[toc] | [next] | [standalone]

#90878

On 2015-05-19, Cecil Westerhof <Cecil@decebal.nl> wrote:
> At the moment I am playing with things like:
>     p = subprocess.Popen('ls -l', shell = True, stdout = subprocess.PIPE)
>
> I think that most of the times this are the values I want. So it would
> be nice to overrule the defaults. What is the best way to do this? So
> creating a function that is exactly the same except for the defaults
> for shell and stdout (and maybe stderr).

Yes.

  def shellprocess(cmd, **kwargs):
      return subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE,
          **kwargs)

> It is a little less important as I first thought, because I found the
> following:
>     error, output = subprocess.getstatusoutput('ls -1')
>     files_new = output.splitlines()
> But it is still nice to know.

Why are you doing this anyway, rather than using os.listdir()?
Invoking subprocesses via the shell is very rarely a good idea.

[toc] | [prev] | [next] | [standalone]

#90892

Op Tuesday 19 May 2015 19:36 CEST schreef Jon Ribbens:

> On 2015-05-19, Cecil Westerhof <Cecil@decebal.nl> wrote:
>> At the moment I am playing with things like: p =
>> subprocess.Popen('ls -l', shell = True, stdout = subprocess.PIPE)
>>
>> I think that most of the times this are the values I want. So it
>> would be nice to overrule the defaults. What is the best way to do
>> this? So creating a function that is exactly the same except for
>> the defaults for shell and stdout (and maybe stderr).
>
> Yes.
>
> def shellprocess(cmd, **kwargs):
> return subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE,
> **kwargs)

Will that not go wrong if I call it with?
    shellprocess('ls -1', shell = False)


>> It is a little less important as I first thought, because I found
>> the following: error, output = subprocess.getstatusoutput('ls -1')
>> files_new = output.splitlines() But it is still nice to know.
>
> Why are you doing this anyway, rather than using os.listdir()?
> Invoking subprocesses via the shell is very rarely a good idea.

I want to rewrite a Bash script into a Python script. The 'ls -1' is
only an example. But Popen and listdir give a different output. The
sorting is different. But I could sort it myself.

Another problem is that I work with a filter later on. But I could do
that with Python also of-course. So maybe I should rethink what I want
to do. ;-)

-- 
Cecil Westerhof
Senior Software Engineer
LinkedIn: http://www.linkedin.com/in/cecilwesterhof

[toc] | [prev] | [next] | [standalone]

#90894

On 2015-05-19, Cecil Westerhof <Cecil@decebal.nl> wrote:
> Op Tuesday 19 May 2015 19:36 CEST schreef Jon Ribbens:
>
>> On 2015-05-19, Cecil Westerhof <Cecil@decebal.nl> wrote:
>>> At the moment I am playing with things like: p =
>>> subprocess.Popen('ls -l', shell = True, stdout = subprocess.PIPE)
>>>
>>> I think that most of the times this are the values I want. So it
>>> would be nice to overrule the defaults. What is the best way to do
>>> this? So creating a function that is exactly the same except for
>>> the defaults for shell and stdout (and maybe stderr).
>>
>> Yes.
>>
>> def shellprocess(cmd, **kwargs):
>> return subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE,
>> **kwargs)
>
> Will that not go wrong if I call it with?
>     shellprocess('ls -1', shell = False)

Yes. I thought you wanted a function that was specific to your
purpose, rather than one that was identical to Popen but with
different defaults. For that I suppose you could do:

  def shellprocess(args, **kwargs):
      kwargs.setdefault("shell", True)
      kwargs.setdefault("stdout", subprocess.PIPE)
      return subprocess.Popen(args, **kwargs)

> I want to rewrite a Bash script into a Python script. The 'ls -1' is
> only an example. But Popen and listdir give a different output. The
> sorting is different. But I could sort it myself.
>
> Another problem is that I work with a filter later on. But I could do
> that with Python also of-course. So maybe I should rethink what I want
> to do. ;-)

Indeed, porting a script from bash to Python is probably best done by
writing the identical functionality in the style of Python, rather
than doing a line-by-line literal conversion.

[toc] | [prev] | [next] | [standalone]

#90895

Op Tuesday 19 May 2015 21:13 CEST schreef Cecil Westerhof:

> Op Tuesday 19 May 2015 19:36 CEST schreef Jon Ribbens:
>
>> On 2015-05-19, Cecil Westerhof <Cecil@decebal.nl> wrote:
>>> At the moment I am playing with things like: p =
>>> subprocess.Popen('ls -l', shell = True, stdout = subprocess.PIPE)
>>>
>>> I think that most of the times this are the values I want. So it
>>> would be nice to overrule the defaults. What is the best way to do
>>> this? So creating a function that is exactly the same except for
>>> the defaults for shell and stdout (and maybe stderr).
>>
>> Yes.
>>
>> def shellprocess(cmd, **kwargs):
>> return subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE,
>> **kwargs)
>
> Will that not go wrong if I call it with?
> shellprocess('ls -1', shell = False)
>
>
>>> It is a little less important as I first thought, because I found
>>> the following: error, output = subprocess.getstatusoutput('ls -1')
>>> files_new = output.splitlines() But it is still nice to know.
>>
>> Why are you doing this anyway, rather than using os.listdir()?
>> Invoking subprocesses via the shell is very rarely a good idea.
>
> I want to rewrite a Bash script into a Python script. The 'ls -1' is
> only an example. But Popen and listdir give a different output. The
> sorting is different. But I could sort it myself.
>
> Another problem is that I work with a filter later on. But I could
> do that with Python also of-course. So maybe I should rethink what I
> want to do. ;-)

It looks like that this does what I want (the dot is needed so that it
also works with 2.7):
    files = sorted(os.listdir('.'))
    p = re.compile('actions-2015-05-[0-9][0-9].sql$')
    current_month = [ file for file in files if p.match(file) ]

-- 
Cecil Westerhof
Senior Software Engineer
LinkedIn: http://www.linkedin.com/in/cecilwesterhof

[toc] | [prev] | [next] | [standalone]

#90897

On 2015-05-19, Cecil Westerhof <Cecil@decebal.nl> wrote:
> It looks like that this does what I want (the dot is needed so that it
> also works with 2.7):
>     files = sorted(os.listdir('.'))
>     p = re.compile('actions-2015-05-[0-9][0-9].sql$')
>     current_month = [ file for file in files if p.match(file) ]

You could instead do (in Python 2 or 3):

  files = glob.glob("actions-2015-05-[0-9][0-9].sql")
  files.sort()

[toc] | [prev] | [next] | [standalone]

#90901

Op Tuesday 19 May 2015 23:28 CEST schreef Jon Ribbens:

> On 2015-05-19, Cecil Westerhof <Cecil@decebal.nl> wrote:
>> It looks like that this does what I want (the dot is needed so that
>> it also works with 2.7): files = sorted(os.listdir('.')) p =
>> re.compile('actions-2015-05-[0-9][0-9].sql$') current_month = [
>> file for file in files if p.match(file) ]
>
> You could instead do (in Python 2 or 3):
>
> files = glob.glob("actions-2015-05-[0-9][0-9].sql")
> files.sort()

Something to remember.

But in this case I also need the previous month. So I have:
    files           = sorted(os.listdir('.'))
    p               = re.compile('actions-2015-05-[0-9][0-9].sql$')
    current_month   = [ file for file in files if p.match(file) ]
    p               = re.compile('actions-2015-04-[0-9][0-9].sql$')
    previous_month  = [ file for file in files if p.match(file) ]

Of-course I will not hard-code the months in the real code.

-- 
Cecil Westerhof
Senior Software Engineer
LinkedIn: http://www.linkedin.com/in/cecilwesterhof

[toc] | [prev] | [next] | [standalone]

#90902

On 2015-05-19 23:23, Cecil Westerhof wrote:
> Op Tuesday 19 May 2015 23:28 CEST schreef Jon Ribbens:
>
>> On 2015-05-19, Cecil Westerhof <Cecil@decebal.nl> wrote:
>>> It looks like that this does what I want (the dot is needed so that
>>> it also works with 2.7): files = sorted(os.listdir('.')) p =
>>> re.compile('actions-2015-05-[0-9][0-9].sql$') current_month = [
>>> file for file in files if p.match(file) ]
>>
>> You could instead do (in Python 2 or 3):
>>
>> files = glob.glob("actions-2015-05-[0-9][0-9].sql")
>> files.sort()
>
> Something to remember.
>
> But in this case I also need the previous month. So I have:
>      files           = sorted(os.listdir('.'))
>      p               = re.compile('actions-2015-05-[0-9][0-9].sql$')
>      current_month   = [ file for file in files if p.match(file) ]
>      p               = re.compile('actions-2015-04-[0-9][0-9].sql$')
>      previous_month  = [ file for file in files if p.match(file) ]
>
> Of-course I will not hard-code the months in the real code.
>
In a regex, '.' will match any character except '\n', or any character
at all if the DOTALL ('(?s)') flag in turned on. If you want to match
an actual '.', you should escape it like this: r'\.'. (And if you're
using backslashes in a string literal, make it a raw string literal!)

     p = re.compile(r'actions-2015-05-[0-9][0-9]\.sql$')

[toc] | [prev] | [next] | [standalone]

#90904

Op Wednesday 20 May 2015 01:20 CEST schreef MRAB:

> On 2015-05-19 23:23, Cecil Westerhof wrote:
>> Op Tuesday 19 May 2015 23:28 CEST schreef Jon Ribbens:
>>
>>> On 2015-05-19, Cecil Westerhof <Cecil@decebal.nl> wrote:
>>>> It looks like that this does what I want (the dot is needed so
>>>> that it also works with 2.7): files = sorted(os.listdir('.')) p =
>>>> re.compile('actions-2015-05-[0-9][0-9].sql$') current_month = [
>>>> file for file in files if p.match(file) ]
>>>
>>> You could instead do (in Python 2 or 3):
>>>
>>> files = glob.glob("actions-2015-05-[0-9][0-9].sql")
>>> files.sort()
>>
>> Something to remember.
>>
>> But in this case I also need the previous month. So I have:
>> files           = sorted(os.listdir('.'))
>> p               = re.compile('actions-2015-05-[0-9][0-9].sql$')
>> current_month   = [ file for file in files if p.match(file) ]
>> p               = re.compile('actions-2015-04-[0-9][0-9].sql$')
>> previous_month  = [ file for file in files if p.match(file) ]
>>
>> Of-course I will not hard-code the months in the real code.
>>
> In a regex, '.' will match any character except '\n', or any
> character at all if the DOTALL ('(?s)') flag in turned on. If you
> want to match an actual '.', you should escape it like this: r'\.'.
> (And if you're using backslashes in a string literal, make it a raw
> string literal!)
>
> p = re.compile(r'actions-2015-05-[0-9][0-9]\.sql$')

Oops, you are correct.

It is now:
    start               = database + '-'
    end                 = r'-[0-9][0-9]\.sql$'
    p                   = re.compile(start + current_month  + end)
    current_month_lst   = [ file for file in files if p.match(file) ]
    p                   = re.compile(start + previous_month + end)
    previous_month_lst  = [ file for file in files if p.match(file) ]

-- 
Cecil Westerhof
Senior Software Engineer
LinkedIn: http://www.linkedin.com/in/cecilwesterhof

[toc] | [prev] | [next] | [standalone]

#90880

[Multipart message — attachments visible in raw view] — view raw

On 19.05.2015 19:01, Cecil Westerhof wrote:
> At the moment I am playing with things like:
>     p = subprocess.Popen('ls -l', shell = True, stdout = subprocess.PIPE)
> 
> I think that most of the times this are the values I want. So it would
> be nice to overrule the defaults. What is the best way to do this? 

I would like to answer with a counter-question: Why do you want to do
this? What is the reason to use ls -l if you could also be using
os.listdir() or os.scandir()?

There are some reasons against screenscraping `ls` (locale-specifities
and environment variable dependencies of ls) and using shell=True (with
shells being huge complex beasts which may have unexpected security
issues, see Shellshock), so I’m not sure why you would want to do that.

regards,
jwi

[toc] | [prev] | [next] | [standalone]

#90881

On Wed, May 20, 2015 at 3:01 AM, Cecil Westerhof <Cecil@decebal.nl> wrote:
> At the moment I am playing with things like:
>     p = subprocess.Popen('ls -l', shell = True, stdout = subprocess.PIPE)
>
> I think that most of the times this are the values I want. So it would
> be nice to overrule the defaults. What is the best way to do this? So
> creating a function that is exactly the same except for the defaults
> for shell and stdout (and maybe stderr).

Well... I would have to start by saying that you probably _don't_ want
to use shell=True by default. Putting it explicitly on the cases where
you need it helps you remember its danger. You also don't need it for
simple cases like that one; improve your reliability by providing a
list instead of a string, and then you can leave shell=False:

p = subprocess.Popen(['ls','-l'], stdout=subprocess.PIPE)

Running everything via the shell is unnecessary, and a dangerous
default. (Maybe it's not a problem when you use a string literal as
the command, but if you make that the default, you'll end up exposing
yourself in some situation where it isn't hard-coded.) With that
change, there's really only one parameter that you're defaulting, so
there's not as much point making the change, but the technique still
works, and maybe you'll add more to the setup:

@functools.wraps(subprocess.Popen)
def Popen(*a, **kw):
    if 'stdout' not in kw: kw['stdout'] = subprocess.PIPE
    return subprocess.Popen(*a, **kw)

That's a simple way to patch in some function defaults. But
personally, I'd probably end up doing something like this:

def capture_stdout(*a, **kw):
    if 'stdout' not in kw: kw['stdout'] = subprocess.PIPE
    return subprocess.Popen(*a, **kw).stdout

so I can directly iterate over the thing. Or something else. One
change isn't really enough to justify the extra layer of indirection.

ChrisA

[toc] | [prev] | [next] | [standalone]

#90883

[Multipart message — attachments visible in raw view] — view raw

On May 19, 2015 12:48 PM, "Chris Angelico" <rosuav@gmail.com> wrote:
>
> On Wed, May 20, 2015 at 3:01 AM, Cecil Westerhof <Cecil@decebal.nl> wrote:
> > At the moment I am playing with things like:
> >     p = subprocess.Popen('ls -l', shell = True, stdout =
subprocess.PIPE)
> >
> > I think that most of the times this are the values I want. So it would
> > be nice to overrule the defaults. What is the best way to do this? So
> > creating a function that is exactly the same except for the defaults
> > for shell and stdout (and maybe stderr).
>
> Well... I would have to start by saying that you probably _don't_ want
> to use shell=True by default. Putting it explicitly on the cases where
> you need it helps you remember its danger. You also don't need it for
> simple cases like that one; improve your reliability by providing a
> list instead of a string, and then you can leave shell=False:
>
> p = subprocess.Popen(['ls','-l'], stdout=subprocess.PIPE)
>
> Running everything via the shell is unnecessary, and a dangerous
> default. (Maybe it's not a problem when you use a string literal as
> the command, but if you make that the default, you'll end up exposing
> yourself in some situation where it isn't hard-coded.) With that
> change, there's really only one parameter that you're defaulting, so
> there's not as much point making the change, but the technique still
> works, and maybe you'll add more to the setup:
>
> @functools.wraps(subprocess.Popen)
> def Popen(*a, **kw):
>     if 'stdout' not in kw: kw['stdout'] = subprocess.PIPE
>     return subprocess.Popen(*a, **kw)
>
> That's a simple way to patch in some function defaults. But
> personally, I'd probably end up doing something like this:
>
> def capture_stdout(*a, **kw):
>     if 'stdout' not in kw: kw['stdout'] = subprocess.PIPE

Just a quick note that this line can be simplified nicely to:

kw.setdefault('stdout', subprocess.PIPE)

Regards,
--
Zach
(on a phone)

[toc] | [prev] | [next] | [standalone]

#90886

On Wed, May 20, 2015 at 3:55 AM, Zachary Ware
<zachary.ware+pylist@gmail.com> wrote:
>> def capture_stdout(*a, **kw):
>>     if 'stdout' not in kw: kw['stdout'] = subprocess.PIPE
>
> Just a quick note that this line can be simplified nicely to:
>
> kw.setdefault('stdout', subprocess.PIPE)

Yes, in the simple case. That does require pre-evaluating
subprocess.PIPE though, which in some situations is an important
distinction. But you're right, for something this simple, the
repetition is superfluous. Good catch.

ChrisA

[toc] | [prev] | [next] | [standalone]

#90909

Cecil Westerhof wrote:

> At the moment I am playing with things like:
>     p = subprocess.Popen('ls -l', shell = True, stdout = subprocess.PIPE)
> 
> I think that most of the times this are the values I want. So it would
> be nice to overrule the defaults. What is the best way to do this?
> So creating a function that is exactly the same except for the defaults
> for shell and stdout (and maybe stderr).

As always in OOP: override the method and call that of the superclass 
in yours.

-- 
PointedEars

Twitter: @PointedEars2
Please do not cc me. / Bitte keine Kopien per E-Mail.

[toc] | [prev] | [standalone]

Back to top | Article view | comp.lang.python

csiph-web