As requested, some constructive criticism of your module. On Thu, 25 Jul 2013 09:24:30 -0400, Devyn Collier Johnson wrote:
> #!/usr/bin/python3 > #Made by Devyn Collier Johnson, NCLA, Linux+, LPIC-1, DCTS What's NCLA, Linux+, LPIC-1, DCTS? Do these mean anything? Are we supposed to know what they mean? "Made by" has no legal significance. You probably want: Copyright © 2013 Devyn Collier Johnson. > #Made using the Geany IDE Nobody gives a monkey's toss what editor you used to type up the module. You might as well mention the brand of monitor you used, or whether the keyboard is Dvorak or Qwerty. > #LGPLv3 You can't just drop in a mention of "LGPLv3" and expect it to mean anything. You actually have to obey the licence yourself, and that includes *actually including the licence in your work*. (You're technically in violation of the licence at the moment, however since the only person whose copyright you are infringing is yourself, it doesn't matter. However anyone else using your code is at risk.) http://www.gnu.org/licenses/gpl-howto.html In the case of the LGPL, you have to include the text of *both* the GPL and the LGPL, not just one. > import re, sys, subprocess, platform > def grep(regex,textf): > #Sample Command: grep.grep("^x",dir()) #Syntax: > boash.grep(regexp_string,list_of_strings_to_search) Comments using # are only of use to people reading the source code. If you want comments to be available at the interactive prompt, you should write them as doc strings: def grep(regex, textf): """This string is a docstring. Sample command: ... Blah blah blah """ Then, at the interactive prompt, the user can say: help(boash.grep) to read the docstring. > version = '0.2a' That's quite useless, since it is a local variable invisible outside of the function. Also, why would you bother giving every individual function a version number? That's rather pointless. The user cannot pick and choose function A with version number 0.6 and function B with version number 0.7 if the module provides versions 0.7 of both. > expr = re.compile(regex) > match = re.findall(expr, textf) > if match != None: > print(match) When comparing with None, it is preferred to use "is" and "is not" rather than equality tests. > def ls(): > version = '0.3' > print(subprocess.getoutput('ls')) > def dir(): > version = '0.3' > print(subprocess.getoutput('dir')) A blank line or two between functions does wonders for readability. There is no prize for conserving newlines. You might like to read PEP 8, the Python style guide. It is optional, but still makes a very good guide. http://www.python.org/dev/peps/pep-0008/ > def bash(*arg): > version = '0.3' > print(subprocess.getoutput(arg)) > def shell(*arg): > version = '0.3' > print(subprocess.getoutput(arg)) bash is not a synonym for "shell". "The shell" might be sh, csh, bash, or any one of many other shells, all of which are slightly (or not so slightly) different. > def clear_bash_history(): > version = '0.3' > print(subprocess.getoutput('history -c')) [...] Do you really need ten aliases for 'history -c'? If you want to define aliases for a function, don't recreate the entire function ten times. Start with defining the function once, then: clear_bash_hist = clear_hist = clear_history = clear_bash_history etc. But really, having ten names for the one function just confuses people, who then wonder what subtle difference there is between delete_history and clear_history. > def firefox(): > version = '0.3' > print(subprocess.Popen('(firefox &)')) Is Firefox really so important that it needs a dedicated command? What about Debian users? Doesn't Iceweasel get a command? > def xterm(): > version = '0.3' > print(subprocess.Popen('(xterm &)')) Surely the user already has an xterm open, if they are running this interactively? Why not just use your xterm's "new window" or "new tab" command? [...delete more trivial calls to subprocess...] > def repeat_cmd(): > version = '0.3' > print(subprocess.Popen('!!')) [... delete two exact copies of this function...] > def ejcd(): > version = '0.3' > print(subprocess.Popen('eject cdrom1')) [... delete FOURTEEN exact copies of this function...] Really? Is anyone going to type "eject_disc_tray" instead of "eject"? I think that will do. This doesn't really do anything except define a large number of trivial wrappers to commands already available in the shell. Emphasis on the *trivial* -- with the exception of the grep wrapper, which is all of four lines (ignoring the useless internal version number), every single one of these wrapper functions is a one-liner.[1] In other words, you're not adding any value to the shell commands by wrapping them in Python. There are plenty of big, complex shell commands that take a plethora of options and could do with some useful Python wrappers, like wget. But you haven't done them. Nor have you added extra security, or even extra convenience. You've done nothing that couldn't be done using the shell "alias" command, except in Python where the syntax is less convenient (e.g. "ls" in the shell, versus "ls()" in Python). [1] I think every newbie programmer goes through a stage of pointlessly writing one-liner wrappers to every second function they see. I know I did. The difference is, before the Internet, nobody did it publicly. -- Steven -- http://mail.python.org/mailman/listinfo/python-list
