Path: csiph.com!aioe.org!newsfeed.tele2net.at!newsfeed.utanet.at!feeder1.cambriumusenet.nl!feed.tweaknews.nl!194.109.133.86.MISMATCH!newsfeed.xs4all.nl!newsfeed3.news.xs4all.nl!xs4all!newsgate.cistron.nl!newsgate.news.xs4all.nl!post.news.xs4all.nl!not-for-mail Return-Path: X-Original-To: python-list@python.org Delivered-To: python-list@mail.python.org X-Spam-Status: OK 0.000 X-Spam-Evidence: '*H*': 1.00; '*S*': 0.00; 'else:': 0.03; 'argument': 0.05; '"""': 0.07; 'class,': 0.07; 'plenty': 0.07; 'python3': 0.07; 'arguments': 0.09; 'arguments,': 0.09; 'collier': 0.09; 'subject:module': 0.09; 'try:': 0.09; 'cc:addr:python-list': 0.11; 'python': 0.11; 'def': 0.12; 'wrote': 0.14; 'amd': 0.16; 'arg': 0.16; 'behave': 0.16; 'command,': 0.16; 'command-line': 0.16; 'command.': 0.16; 'edt': 0.16; 'itself,': 0.16; 'likewise': 0.16; 'ls:': 0.16; 'non-trivial': 0.16; 'printout': 0.16; 'self.args': 0.16; 'somehow,': 0.16; 'true:': 0.16; 'x86_64': 0.16; '\xa0def': 0.16; '\xa0you': 0.16; 'wrote:': 0.18; 'module': 0.19; 'thu,': 0.19; 'user.': 0.19; 'not,': 0.20; '8bit%:5': 0.22; 'command': 0.22; '>>>': 0.22; 'import': 0.22; 'coding': 0.22; 'email addr:gmail.com>': 0.22; 'shell': 0.22; 'cc:addr:python.org': 0.22; '>>>': 0.24; 'module,': 0.24; 'passes': 0.24; 'processor': 0.24; 'skip:\xa0 20': 0.24; '\xa0if': 0.24; 'looks': 0.24; 'cc:2**0': 0.24; '>': 0.26; 'this:': 0.26; 'pass': 0.26; 'certain': 0.27; 'header:In-Reply-To:1': 0.27; 'idea': 0.28; 'point': 0.28; 'skip:p 30': 0.29; 'am,': 0.29; 'unix': 0.29; '(like': 0.30; 'message-id:@mail.gmail.com': 0.30; 'program,': 0.31; 'code': 0.31; 'getting': 0.31; "skip:' 10": 0.31; '25,': 0.31; 'directly,': 0.31; 'omitted': 0.31; 'pipe': 0.31; 'allows': 0.31; 'file': 0.32; 'class': 0.32; 'lists': 0.32; 'linux': 0.33; 'framework': 0.33; 'style': 0.33; 'skip:_ 10': 0.34; 'problem': 0.35; 'basic': 0.35; 'advice': 0.35; 'except': 0.35; 'possible.': 0.35; 'something': 0.35; 'johnson': 0.35; 'but': 0.35; 'received:google.com': 0.35; '8bit%:9': 0.36; 'false': 0.36; 'module.': 0.36; 'method': 0.36; "i'll": 0.36; 'possible': 0.36; 'list': 0.37; 'easily': 0.37; 'feedback': 0.38; 'skip:& 10': 0.38; '8bit%:4': 0.38; 'fri': 0.38; 'jason': 0.38; 'does': 0.39; 'skip:p 20': 0.39; 'called': 0.40; 'users': 0.40; 'even': 0.60; 'commands': 0.60; 'break': 0.61; "you're": 0.61; "you've": 0.63; 'real': 0.63; 'such': 0.63; '8bit%:10': 0.64; 'provide': 0.64; 'more': 0.64; 'to:addr:gmail.com': 0.65; 'close': 0.67; 'gotten': 0.74; 'jul': 0.74; 'complexity': 0.84; 'url:ls': 0.84; 'contents.': 0.91; 'do:': 0.91; '2013': 0.98 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=iMQLxtJh28xor8k2G3lmB5+k1ts5Hg39wt2ME60vx/I=; b=0o74bFj1AJiHEzP3Lrz8myI1dfIJ+mYnGhbEtMVUcwAXzMpb3WcN2lVg16pJPKaWWs 2pSGuSxAeviBPMOrFPv5poMqfaWiP7QpbEWOjvuxCSDMrAqxNw4dRDmaD+u8M6mJE8M9 1rwL90cz4LOWk16PNfsOAp8Eb5juxa+1G0opU8rJkWlcV9JqAeLcvtAiATf4FjSIbvtK eecbV10zeu99HSkyLQBtI/6GnoAdLyybi8pmoV+uetIaUbEgJtWPQmAqKrowRc0rDwCD c8UuV6d+Wc91aqfHDQ29IDDE86BFUZDmtK2gHd5mNgUOoO/mB8+czlGrwYiAem67Rd7V g8AA== MIME-Version: 1.0 X-Received: by 10.42.228.1 with SMTP id jc1mr21284758icb.92.1374938097581; Sat, 27 Jul 2013 08:14:57 -0700 (PDT) In-Reply-To: <51F1270E.6000704@Gmail.com> References: <51F1270E.6000704@Gmail.com> Date: Sat, 27 Jul 2013 11:14:57 -0400 Subject: Re: Critic my module From: Jason Swails To: Devyn Collier Johnson Content-Type: multipart/alternative; boundary=001a1133d7b8e5041004e27fb89b Cc: Python Mailing List X-BeenThere: python-list@python.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: General discussion list for the Python programming language List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Newsgroups: comp.lang.python Message-ID: Lines: 218 NNTP-Posting-Host: 2001:888:2000:d::a6 X-Trace: 1374938107 news.xs4all.nl 15962 [2001:888:2000:d::a6]:38483 X-Complaints-To: abuse@xs4all.nl Xref: csiph.com comp.lang.python:51362 --001a1133d7b8e5041004e27fb89b Content-Type: text/plain; charset=ISO-8859-1 You've gotten plenty of good advice from people discussing the coding and coding style itself, I'll provide some feedback from the vantage point of a perspective user. On Thu, Jul 25, 2013 at 9:24 AM, Devyn Collier Johnson < devyncjohnson@gmail.com> wrote: > Aloha Python Users! > > I made a Python3 module that allows users to use certain Linux shell > commands from Python3 more easily than using os.system(), > subprocess.Popen(), or subprocess.getoutput(). This module (once placed > with the other modules) can be used like this > > import boash; boash.ls() > I actually wrote a program recently in which I wanted access to unix "ls" command, and I wanted it to behave as close to the real, UNIX "ls" as possible. This would seem like a perfect use-case for your module, but the problem is that the 'ls' command in your module does not behave much like the real 'ls' command. You never let any of the 'system' commands in your module access any arguments. More often than not, I use "ls" with several command-line arguments, like: ls --color=auto -lthr dir_basename*/ Even if you're just spawning 'ls' directly, this is actually non-trivial to implement. You need globbing on all non-option arguments, you may want to pass up the return code somehow, depending on what the user wants to do: [bash ]$ ls nodir ls: nodir: No such file or directory [bash ]$ echo $? 1 Also, 'ls' in the terminal behaves like "ls -C" when called from your module. In the framework of my program, my 'ls' command looks like this: class ls(Action): """ Lists directory contents. Like UNIX 'ls' """ needs_parm = False def init(self, arg_list): from glob import glob self.args = [] # Process the argument list to mimic the real ls as much as possible while True: try: arg = arg_list.get_next_string() if not arg.startswith('-'): # Glob this argument globarg = glob(arg) if len(globarg) > 0: self.args.extend(globarg) else: self.args.append(arg) else: self.args.append(arg) except NoArgument: break def __str__(self): from subprocess import Popen, PIPE process = Popen(['/bin/ls', '-C'] + self.args, stdout=PIPE, stderr=PIPE) out, err = process.communicate('') process.wait() return out + err [I have omitted the Action base class, which processes the user command-line arguments and passes it to the init() method in arg_list -- this listing was just to give you a basic idea of the complexity of getting a true-er 'ls' command]. Your 'uname' command is likewise limited (and the printout looks strange: >>> print(platform.uname()) ('Linux', 'Batman', '3.3.8-gentoo', '#1 SMP Fri Oct 5 14:14:57 EDT 2012', 'x86_64', 'AMD FX(tm)-6100 Six-Core Processor') Whereas: [bash $] uname -a Linux Batman 3.3.8-gentoo #1 SMP Fri Oct 5 14:14:57 EDT 2012 x86_64 AMD FX(tm)-6100 Six-Core Processor AuthenticAMD GNU/Linux You may want to change that to: def uname(): print(' '.join(platform.uname())) Although again, oftentimes people want only something specific from uname (like -m or -n). HTH, Jason --001a1133d7b8e5041004e27fb89b Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable
You've gotten plenty of good advice from peopl= e discussing the coding and coding style itself, I'll provide some feed= back from the vantage point of a perspective user.


On Thu, Jul 25, 2013 at 9:24 AM, Devyn C= ollier Johnson <devyncjohnson@gmail.com> wrote:
Aloha Python Users!

=A0 =A0I made a Python3 module that allows users to use certain Linux shell= commands from Python3 more easily than using os.system(), subprocess.Popen= (), or subprocess.getoutput(). This module (once placed with the other modu= les) can be used like this

import boash; boash.ls()<= br>

I actually wrote a program recently in which I wanted access to= unix "ls" command, and I wanted it to behave as close to the rea= l, UNIX "ls" as possible.

This would seem like a perfe= ct use-case for your module, but the problem is that the 'ls' comma= nd in your module does not behave much like the real 'ls' command. = =A0You never let any of the 'system' commands in your module access= any arguments. =A0More often than not, I use "ls" with several c= ommand-line arguments, like:

ls --color=3Dauto -lthr dir_= basename*/
Even if you're just spawning 'ls' directly, this is actually no= n-trivial to implement. =A0You need globbing on all non-option arguments, y= ou may want to pass up the return code somehow, depending on what the user = wants to do:

[b= ash ]$ ls nodir
ls: nodir: No such file or directory
[bash ]$ echo $?
1
<= div class=3D"gmail_default">
Also, 'ls' in the terminal behaves like &qu= ot;ls -C" when called from your module. =A0In the framework of my prog= ram, my 'ls' command looks like this:

class ls(Action):
=A0 =A0"""
=A0 =A0Lists directory contents. Like UNIX 'ls'
=A0 =A0"""
= =A0 =A0needs_parm =3D False
=A0 =A0def in= it(self, arg_list):
=A0 =A0 =A0 from glob import glob
=A0 =A0 =A0 self.args =3D []
=A0 =A0 =A0 # Process the argument list to mimic the real ls as m= uch as possible
=A0 =A0 =A0 while True:
=A0 =A0 =A0 =A0 = =A0try:
=A0 =A0 =A0 =A0 =A0 =A0 arg =3D a= rg_list.get_next_string()
=A0 =A0 =A0 =A0= =A0 =A0 if not arg.startswith('-'):
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0# Glob this arg= ument
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0glob= arg =3D glob(arg)
=A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0if len(globarg) > 0:
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 self.args.extend(globarg)
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0else:
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 self.args.append(arg= )
=A0 =A0 =A0 =A0 =A0 =A0 else:
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0self.args.appen= d(arg)
=A0 =A0 =A0 =A0 =A0except NoArgume= nt:
=A0 =A0 =A0 =A0 =A0 =A0 break

=A0 =A0def __str__(self):
=A0 =A0 =A0 fro= m subprocess import Popen, PIPE
=A0 =A0 = =A0 process =3D Popen(['/bin/ls', '-C'] + self.args, stdout= =3DPIPE, stderr=3DPIPE)
=A0 =A0 =A0 out, err =3D process.communicate(&= #39;')
=A0 =A0 =A0 process.wait()
=A0 =A0 =A0 return out + err

<= /div>
[I have omitted the Action base class, which processes the = user command-line arguments and passes it to the init() method in arg_list = -- this listing was just to give you a basic idea of the complexity of gett= ing a true-er 'ls' command].

Your 'uname' command is likewise li= mited (and the printout looks strange:

>>> print(platform.uname())
('Linux', '= ;Batman', '3.3.8-gentoo', '#1 SMP Fri Oct 5 14:14:57 EDT 20= 12', 'x86_64', 'AMD FX(tm)-6100 Six-Core Processor')

Whereas:

[ba= sh $] uname -a
Linux Batman 3.3.8-gentoo #1 SMP Fri Oc= t 5 14:14:57 EDT 2012 x86_64 AMD FX(tm)-6100 Six-Core Processor AuthenticAM= D GNU/Linux

You may want to change that to:
<= br>
def uname():
=A0 =A0 print(' '.= join(platform.uname()))

Although again= , oftentimes people want only something specific from uname (like -m or -n)= .

HTH,
Jason
=
--001a1133d7b8e5041004e27fb89b--