What is a good Python format style

Question

I've created a small python script to toggle between two files I'm using for testing.

My question is, what is a good Python format style for the following code:

import filecmp
import shutil
local = "local.txt"
remote = "remote.txt"
config_file = "C:\some\path\file.txt"
shutil.copyfile( remote if( filecmp.cmp(local, config_file ) ) else local, config_file )

Or

shutil.copyfile( remote 
 if( filecmp.cmp(local, config_file ) ) 
 else local,
 config_file )

Or

tocopy = remote if( filecmp.cmp( local, config_file ) ) else local 
shutil.copyfile( tocopy, config_file )

Or what?

Also, what is the preferred way to name var in python for many-word names, is it "to_copy", "tocopy", "toCopy", "ToCopy"

• S.Lott: There are a number of questions on related topics. See stackoverflow.com/questions/159720/… for example.
• OscarRyz: Actually the question was mo tin the if:else: "ternary" for python, but once here, why not re-ask for var names. Thaks for the link though
• Brian C. Lane: To me the 'y if x else z' is just way too perlish for me. Split it out into an if/else like Greg has shown.
• Brian: Careful with the backslashes - remember these are treated as escapes, so if your path was "C:\some\path\test.txt" you'll be scratching your head until you realise its trying to open "path[TAB]est.txt". Better to use forward slashes, or failing that, escape using \\ or raw strings.
• OscarRyz: @Brian: Really? I though this was an advantage over other languages. Do you have any idea of why did this work ? ( didn't it? :-/ )

Answer 1

For the conditional statement, I would probably go with:

if filecmp.cmp(local, config_file):
 shutil.copyfile(remote, config_file)
else:
 shutil.copyfile(local, config_file)

There's little need to use the inline y if x else z in this case, since the surrounding code is simple enough.

• Vinko Vrsalovic: So are you saying that y if x else z is to be used when the code is complex?
• OscarRyz: I would think the opposite, When the code is complex I would rather see each thing in its own if:else: branch. :) well mmhhh at least for java, probably python style is different in this case :)
• S.Lott: @Vinko Vrsalovic: I agree with the comment that an inline "if" makes the statement harder to read. I think "trivial" might be a poor choice of words. But the inline "if" in the original post is hard to read.
• Greg Hewgill: I meant that the surrounding code is simple enough to not worry about repeating it (DRY principle). If it were a more complex expression I would be concerned about duplicating too much code, and would probably use temporary variables instead (like your third proposal).
• Vinko Vrsalovic: I prefer to be dry, either by paying the cost of a ternary, or by encapsulating the shutil.copyfile

Answer 2

From the Python Style Guide:

With regard to listing out a compound expression:

Compound statements (multiple statements on the same line) are generally discouraged.

Yes:

if foo == 'blah':
 do_blah_thing()
do_one()
do_two()
do_three()

Or for the code you supplied, Greg's example is a good one:

if filecmp.cmp(local, config_file):
 shutil.copyfile(remote, config_file)
else:
 shutil.copyfile(local, config_file)

Rather not:

if foo == 'blah': do_blah_thing()
do_one(); do_two(); do_three()

Method Names and Instance Variables

Use the function naming rules: lowercase with words separated by underscores as necessary to improve readability.

Update: Per Oscar's request, also listed how his code would look in this fashion.

Answer 3

The third option looks the most natural to me, although your use of spaces in side parentheses and superfluous parentheses contradict the Python style guide.

That guide also answers the to_copy question, but I would probably use clearer names altogether.

I would write it as:

import filecmp
import shutil
local = "local.txt"
remote = "remote.txt"
destination = r"C:\some\path\file.txt"
source = remote if filecmp.cmp(local, destination) else local
shutil.copyfile(source, destination)

• Justin Standard: If you are curious, the leading r in the destination string indicatses that it is a "raw string". So, yes, it was on purpose. So... don't remove it ;)
• Vinko Vrsalovic: LOL! He's proposing the same solution I have proposed and he gets 3 upvotes instead. Amazing :-)
• Justin Standard: I want to add, that even though the raw string was intentional, I really don't think this is the right solution. Check out Greg Hewgill's answer for one a bit more correct.

Answer 4

The most common naming I've seen is underscode separated words, to_copy.

As for the format style, I've seen no such agreement. I find

source = remote if filecmp.cmp(local, config_file) else local
shutil.copyfile(source, config_file)

to be the clearest among your options.

And seeing that everyone prefers to split the if I'd, at the very least, encapsulate the copyfile call in case you someday wish to change it:

def copy_to(source, destination):
 shutil.copyfile(source,destination)
if filecmp.cmp(local, config_file):
 copy_to(remote, config_file)
else:
 copy_to(local, config_file)

• OscarRyz: What is the reason to down_vote this? I mean, what is wrong with that style?
• Patrick Harrington: You're asking about Python style, and this doesn't follow Python's Style Guide - with Python, readability tends to supersede conciseness.
• Vinko Vrsalovic: People don't like it, probably because the ternary is very new and it hurts old pythonistas' eyes. I find it clear enough to avoid repeating the copyfile instead.
• Martin Vilcans: There's no point of moving shutil.copyfile to a separate function. It adds one line of code and does nothing to help comprehension. If you later on wish to change how you copy file, then well, it's not like the last 4 lines in your code are read-only.
• Vinko Vrsalovic: Sigh. The idea is not to help comprehension. It's to help maintainability. Sure it's not hard to modify two lines instead of one, but it's certainly easier to modify only one, especially if you'll replace it for two or three or five...

Answer 5

What about:

import filecmp
import shutil
local = "local.txt"
remote = "remote.txt"
config_file = "C:\some\path\file.txt"
if filecmp.cmp( local, config_file):
 to_copy = remote
else:
 to_copy = local
shutil.copyfile( to_copy, config_file )

yikes, this open id screen name looks terrible.

• Patrick Harrington: Yeah, shouldn't have skipped out on giving yourself a name / nickname. :)