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"

There are a number of questions on related topics. See http://stackoverflow.com/questions/159720/what-is-the-naming-convention-in-python-for-variable-and-function-names for example. — S.Lott, Dec 01 '08 at 19:03
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 — OscarRyz, Dec 01 '08 at 19:17
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 C. Lane, Dec 02 '08 at 01:02
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. — Brian, Dec 02 '08 at 02:06
@Brian: Really? I though this was an advantage over other languages. Do you have any idea of why did this work ? ( didn't it? :-/ ) — OscarRyz, Dec 03 '08 at 03:24
It worked here because none of the string "\s", "\p" or "\f" are valid escape sequences. If you happened to have the first character after the \ be "n" ("\n" = newline) or "t" ("\t"=tab), it would have failed. (Which can be very puzzling when just changing the filename somehow breaks your code) — Brian, Dec 04 '08 at 00:48
:o True!!.. I have C:\path in the posted code, but in my machine the path didn't have any of the mentioned letters. Good call! Additional question, can I use "/" as in Java? for instance this declaration works on Java: new File("C:/some/path/file.txt"); — OscarRyz, Dec 04 '08 at 17:49

score 16 · Accepted Answer · answered Dec 01 '08 at 18:44

16

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.

answered Dec 01 '08 at 18:44

Greg Hewgill

951,095
183
1,149
1,285

So are you saying that y if x else z is to be used when the code is complex? – Vinko Vrsalovic Dec 01 '08 at 18:55
1

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 :) – OscarRyz Dec 01 '08 at 18:58
@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. – S.Lott Dec 01 '08 at 19:01
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). – Greg Hewgill Dec 01 '08 at 19:18
I prefer to be dry, either by paying the cost of a ternary, or by encapsulating the shutil.copyfile – Vinko Vrsalovic Dec 01 '08 at 20:32
@Vinko Vrsalovic: While DRY is highly desirable, I don't think it adds value here. Repeating your interface in XML is the craziness that DRY prevents. Repeating a function name a few lines of code later isn't craziness. A local variable is preferable to an in-line if. – S.Lott Dec 01 '08 at 20:55
Isn't craziness, but isn't necessary either. I'm advocating a local variable or encapsulation, see my disliked answer below. – Vinko Vrsalovic Dec 01 '08 at 21:06
S.Lott, I disagree. I find an inline `if` clearer than writing to and then reading from a temporary variable. – Svante Dec 01 '08 at 23:46
"writing to a variable" is a phrase that doesn't mean much in an OO context. The object will be created as a separate statement or an expression. Assigning a name to that object seems to clarify, not obscure. – S.Lott Dec 02 '08 at 00:51

Vinko Vrsalovic · Answer 2 · 2008-12-01T23:17:51.230

5

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)

edited Dec 01 '08 at 23:17

answered Dec 01 '08 at 18:44

Vinko Vrsalovic

330,807
53
334
373

What is the reason to down_vote this? I mean, what is wrong with that style? – OscarRyz Dec 01 '08 at 18:59
You're asking about Python style, and this doesn't follow Python's Style Guide - with Python, readability tends to supersede conciseness. – Patrick Harrington Dec 01 '08 at 19:37
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. – Vinko Vrsalovic Dec 01 '08 at 19:38
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. – Martin Vilcans Dec 01 '08 at 22:53
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... – Vinko Vrsalovic Dec 01 '08 at 23:12
(imagine now we'll copy the file to a remote location over SSH). I cannot believe people don't get this. Aditionally, you are assuming this happens only in one place in the code, and there's no such guarantee. – Vinko Vrsalovic Dec 01 '08 at 23:14
I really don't get why this has been voted down either. +1 for assigning the ternary result to a named variable. If I could, I'd +1 for the use of a named function to clearly denote intent as well. I tend to do the same thing but with lambdas. – Matthew Trevor Dec 02 '08 at 10:21

Patrick Harrington · Answer 3 · 2008-12-01T19:14:46.277

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.

score 5 · Answer 4 · edited Dec 01 '08 at 21:41

5

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)

edited Dec 01 '08 at 21:41

Justin Standard

21,347
22
80
89

answered Dec 01 '08 at 19:27

Tim Lesher

6,341
2
28
42

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 ;) – Justin Standard Dec 01 '08 at 21:42
LOL! He's proposing the same solution I have proposed and he gets 3 upvotes instead. Amazing :-) – Vinko Vrsalovic Dec 01 '08 at 23:17
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. – Justin Standard Dec 01 '08 at 23:24

score 0 · Answer 5 · answered Dec 01 '08 at 19:33

0

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.

answered Dec 01 '08 at 19:33

Yeah, shouldn't have skipped out on giving yourself a name / nickname. :) – Patrick Harrington Dec 01 '08 at 19:35

What is a good Python format style

5 Answers5

Linked