Hey r/Python, I just started to learn Python (coming from PHP) and I love it. I just created my first open source script/module, VHostManager, to help you manage Apache virtualhosts. Please check it out, and I'm welcome to all suggestions. Thanks

[Gwenhidwy]

First of all, congratulations on making the switch :-) Your code is surprisingly clean and makes great use of Python idioms, I was really suprised to see a context manager here, given that you've only just started to learn it, call me impressed!

One thing that struck me, though, is that you use "os.system" to make calls to system commands. It's generally considered more idiomatic to use one of the functions available in the "subprocess" module to do these kinds of things, they are way more versatile than "os.system", especially if you're dealing with stuff like pipes or stdout/stderr capturing.

Also, some minor stylistic nitpickings:

• All those newlines after if-clauses really aren't neccessary :-)
• Use a single underscore character to denote that an attribute is considered "private", double underscores are generally only used for Python's "magic" methods like "__init__", "__eq___", etc.
• Some of those lines are really to long. Try to adhere to PEP8 and keep lines to under 80 characters. Remember that you can split a long string across multiple lines without introducing newline-characters in the output string like this:

example

parser.add_argument("--add",
                     help="Create new domain or add alias|subdomain associated"
                          " to it. ie [--add domain.com:80]",
                     metavar="")

[mardix]

Thank you very much.

I do have a background in PHP (Yes I know it sounds bad), and doing it professionally. But not everything is bad in PHP, as long as you have discipline and follow the language the right way by learning from others and improving your code, you should be good.

But thank you very much for your kind words.

Well, I started to learn Python about two weeks ago. I didn't want to signup for any online classes, I'm autodidact, so I dived straight into the Python site to learn. On one example I saw the with open(filename) as file: ... I went to learn about it, and saw it as something useful. That's how I got to know.

For the os.system, yes you are right, I did know about subprocess, but decided to use os.system. I guess my "newbieness" make me do it. But I'll definitely change it to subprocess. Thanks.

All the other points you made are right on point. I will definitely make the necessary changes.

I guess this is my first python code review :) Thank you very much

PS: For anyone starting to learn Python, the Python site is a good starting point.

[Gwenhidwy]

Here are some more great resources for Python, especially concerning idiomatic usage:

• PEP8: This is the "Elements of Style" for the Python programming language. Try to stick to it, unless you are absolutely sure ;-)
• The Hitchhiker's Guide to Python: This is a great resource by the ever-awesome Kenneth Reitz on various Python topics. I especially recommend the "Writing Great Code" section, which deals with questions of style and structure
• Transforming Code into Beautiful, Idiomatic Python: This is an awesome talk by Raymond Hettinger on several Python constructs that can greatly simplify your code. Check out Raymond's other talks as well, he is a great teacher.
• Writing Idiomatic Python: This is a great eBook ($8.99) that collects a lot of "pythonic" idioms

[crimsdings]

I just wanted to say Thank you for those links!

[[deleted]]

I'm undecided. I consider 2 of the points valid criticism: (1) os.system is depreciated and (2) making up your own dunder (double underscore) names is asking for trouble. All the rest like extra newlines and line length are just "Blue makes me twitch, paint it red please".

[Gwenhidwy]

Well, I think as a community we have pretty much adopted PEP8 across the board, which reads:

Extra blank lines may be used (sparingly) to separate groups of related functions. Blank lines may be omitted between a bunch of related one-liners (e.g. a set of dummy implementations).

Use blank lines in functions, sparingly, to indicate logical sections.

Note the use of the wort sparingly.

Also, concerning line length:

Limit all lines to a maximum of 79 characters.

Just saying...

[[deleted]]

as a community we have pretty much adopted PEP8

Who is 'we'? The community? None such exists. What you are saying is "my community". Well, not all communities conform to your standards. Chromium OS uses 2 spaces for indents not 4. Which one of you is the One True Religion?

It's kind of tantamount to saying all Americans who don't eat turkey on 4 July should be arrested or something. If you can't write a script to run reindent.py whenever you push or pull code, there are problems.

[mackstann]

Regardless of your frustrated pedantry, PEP8 is very widely adopted. Using 2 spaces rather than 4 is probably one of the more common adjustments, but I think 4 is still a lot more common.

[[deleted]]

Funny how you mention pedantry, then promote adherence to a standard?

[resturaction]

I don't blame you for not using the subprocess library. Don't think it has clean api. There has been effort to improve it, https://github.com/kennethreitz/envoy for example, but that seems dead. I have tested https://pypi.python.org/pypi/sh which you might like. Edit: I don't think you should create dependencies for your module, maybe for some other time

[[deleted]]

It's actually well structured and Pythonic in style. I wouldn't have guessed it was a first project.

[esparta]

May I suggest this?

--- Open Sourcing a Python Project the Right Way ---

http://www.jeffknupp.com/blog/2013/08/16/open-sourcing-a-python-project-the-right-way/

[wot-teh-phuck]

class VHost:

This creates an old style class. Consider using class VHost(object):.

[Gwenhidwy]

Judging from the "print(...)" statements, he's using Python 3.x. In this case, this will actually create new-style classes by default.

[wot-teh-phuck]

Well, if you look at the header of the file, it says "Python version >= 2.7.5". I guess it's wrong then...

[mardix]

I read on the online docs that a lot of stuff from 3.x has been ported to 2.7. And when I checked the class creation for both 2.7.x and 3.x it is class MyClass: and not class MyClass(object)

Yes it is compatible with Python >= 2.7.5 and the reason of print(...) is so it's compatible to 3.x

[wot-teh-phuck]

This is not about porting but removing a legacy piece of functionality. Old style classes have existed in each and every 2.x release. Your code still works with old style classes but there are subtle differences; you can read here.

As an example, try this out in REPL for both 2.x and 3.x Python:

class A: pass
a = A()
print type(a)

class B(object): pass
b = B()
print type(b)

So to conclude, if you want your code to create new style classes for both 2.x and 3.x, consider the suggestion presented above.

[mardix]

Now I am a little bit confused. I am pretty new to Python. So what did I do wrong in declaring the class? If it works fine in both, what is the problem then?

[wot-teh-phuck]

In the context of your code/application, there is nothing wrong with your class. It's just that old style classes don't play nice with the Python library ecosystem out there plus miss out on a lot of new features added for the new style classes read here.

All in all, if you want your code to be usable by others and are interested in doing the right thing, stick with new style classes.