Allowing comments after the line continuation backslash

Y

Yingjie Lan

Hi,

Sorry if I am baking too many ideas today.
I am just having trouble with the backslashes....

I would like to have comments after the line continuation backslash.
and b > 0:
#do something here

This is currently not OK, but this might be a good thing to have.

Yingjie
 
M

Martin v. Loewis

Sorry if I am baking too many ideas today.
I am just having trouble with the backslashes....

I would like to have comments after the line continuation backslash.

and b > 0:
#do something here

This is currently not OK, but this might be a good thing to have.

Can you say why it would be a good idea?
FWIW, I would write your fragment as

if (a > 0 #comments for this condition
and b > 0):
#do something here

i.e. avoid the backslash for multi-line conditions altogether
(in fact, I can't think any situation where I would use the backslash).

Regards,
Martin
 
U

Ulrich Eckhardt

Yingjie said:
I would like to have comments after the line continuation backslash.

and b > 0:
#do something here

Historically, and also in Python, the backslash escapes the immediately
following character. That means that _nothing_ may follow the backslash and
that the backslash is not a "line continuation backslash" per se.

Now, there are ways around this, just add parens:

if (a > 0
and b > 0):

Note that this also applies to (), [] and {} when forming tuples, lists and
dicts, which you can spread across different lines.

This is currently not OK, but this might be a good thing to have.

I'd actually argue that even the currently used backslash is a bad idea to
have.

Uli
 
S

Steven D'Aprano

Hi,

Sorry if I am baking too many ideas today. I am just having trouble with
the backslashes....

Then don't use them.

If you are having so much trouble keeping your lines short, then I
suggest you're trying to do too much in one line.


I would like to have comments after the line continuation backslash.

and b > 0:
#do something here

I'm actually sympathetic to the idea of having a backslash followed by
any whitespace mean line continuation, allowing comments after the line.
The problem is, what should you do here? Presumably if you follow the
backslash with anything other than whitespace or a comment, you get a
SyntaxError:

if a > 0 \ and b > 0:
# do something


In any case, there is a moratorium on language changes, so it will
probably be 2 or 3 years before this could even be considered.

What you can do instead:

if a > 0 and b > 0:
# comment for a condition
# comment for b condition
do_something()

There's nothing wrong with that. But if you insist on in-line comments:

if (a > 0 # comment for a condition
and b > 0 # comment for b condition
):
do_something()


But best of all, write literate code that doesn't need comments because
it documents itself:

if number_of_pages > 0 and not all(page.is_blank() for page in pages):
do_something()
 
N

Neil Cerutti

i.e. avoid the backslash for multi-line conditions altogether
(in fact, I can't think any situation where I would use the
backslash).

The horrible-to-nest with statement.
 
L

Lawrence D'Oliveiro

(in fact, I can't think any situation where I would use the backslash).

for \
Description, Attr, ColorList \
in \
(
("normal", "image", MainWindow.ColorsNormalList),
("highlighted", "highlight", MainWindow.ColorsHighlightedList),
("selected", "select", MainWindow.ColorsSelectedList),
) \
:
...
#end for

<http://github.com/ldo/dvd_menu_animator>
 
C

Chris Rebert

   for \
       Description, Attr, ColorList \
   in \
       (
           ("normal", "image", MainWindow.ColorsNormalList),
           ("highlighted", "highlight", MainWindow.ColorsHighlightedList),
           ("selected", "select", MainWindow.ColorsSelectedList),
       ) \
   :
      ...
   #end for

<http://github.com/ldo/dvd_menu_animator>

I find the level of deviation from PEP 8 in that file rather disturbing.
In any case, the backslashes are easily avoided, and readability
improved IMHO, via refactoring:

desc_attr_colors_triples = (("normal", "image", MainWindow.ColorsNormalList),
("highlighted", "highlight", MainWindow.ColorsHighlightedList),
("selected", "select", MainWindow.ColorsSelectedList))
for in description, attr, color_list in desc_attr_colors_triples:
...

Cheers,
Chris
 
L

Lawrence D'Oliveiro

Chris said:
desc_attr_colors_triples = (("normal", "image",
MainWindow.ColorsNormalList),
("highlighted", "highlight", MainWindow.ColorsHighlightedList),
("selected", "select", MainWindow.ColorsSelectedList))
for in description, attr, color_list in desc_attr_colors_triples:
...

And so you have managed to separate one set of looping conditions into two
parts. What is the significance of the name “desc_attr_colors_triples� None
at all. What purpose does it serve? None, really. Does it ease the
maintenance burden? No, but by splitting your attention across two places,
it actually adds to it.

If this is all your PEP-8 can achieve, then a pox on it.
 
S

Steven D'Aprano

for \
Description, Attr, ColorList \
in \
(
("normal", "image", MainWindow.ColorsNormalList),
("highlighted", "highlight",
MainWindow.ColorsHighlightedList), ("selected", "select",
MainWindow.ColorsSelectedList),
) \
:
...
#end for


If it were your intention to show why backslashes should be avoided, you
succeeded admirably.

The above can be written much more cleanly as:

for Description, Attr, ColorList in (
("normal", "image", MainWindow.ColorsNormalList),
("highlighted", "highlight", MainWindow.ColorsHighlightedList),
("selected", "select", MainWindow.ColorsSelectedList),
):
pass


with no backslashes required. An even better way would be to given the
tuples descriptive names, so that anyone maintaining this software can
easily see what they are for:

# States should be tuples (description, attribute name, colour list).
standard = ("normal", "image", MainWindow.ColorsNormalList)
highlighted = ("highlighted", "highlight",
MainWindow.ColorsHighlightedList)
selected = ("selected", "select", MainWindow.ColorsSelectedList)
for desc, attr, color_list in (standard, highlighted, selected):
pass
 
R

Roy Smith

Chris Rebert said:
I find the level of deviation from PEP 8 in that file rather disturbing.
In any case, the backslashes are easily avoided, and readability
improved IMHO, via refactoring:

desc_attr_colors_triples = (("normal", "image", MainWindow.ColorsNormalList),
("highlighted", "highlight", MainWindow.ColorsHighlightedList),
("selected", "select", MainWindow.ColorsSelectedList))
for in description, attr, color_list in desc_attr_colors_triples:
...

I like and use PEP-8. At the start of any project involving myself,
other people, and Python, I'll generally suggest we follow PEP-8 style,
and I can't remember ever getting any pushback. That being said, I
don't hold it in awe. Likewise, I don't worry in the least about
deviating when readability would be improved by doing so.

In this case, I think I would do:

styles = [("normal", "image", MainWindow.ColorsNormalList),
("highlighted", "highlight", MainWindow.ColorsHighlightedList),
("selected", "select", MainWindow.ColorsSelectedList)]

for in description, attr, color_list in styles:
blah, blah, blah

For those reading this in a non-fixed width font, I've laid out the
definition of styles as a table, with spaces inserted to make the
columns line up. For data like this, I think it makes it easier to read
and comprehend. As a minor nit, note that I made it a list of tuples,
not a tuple of tuples.

I'm tempted to do an additional refactoring to get rid of the verbose
color list names:

CL_Normal = MainWindow.ColorsNormalList)
CL_Highlighted = MainWindow.ColorsHighlightedList
CL_Selected = MainWindow.ColorsSelectedList

styles = [("normal", "image", CL_Normal),
("highlighted", "highlight", CL_Highlighted),
("selected", "select", CL_Selected)]

I haven't decided if this makes things better or worse. For this small
table, I'm inclined to say worse. If the table were much larger and I
were reusing many of the color list names over and over, I would
certainly do that. If MainWindow were a well-designed module and I
could do

import * from MainWindow

without cluttering up my namespace too much, I would do that, then just
use the unadorned names.

Also, depending on what I was doing inside the loop, I might pick
shorter names. For example:

for in d, a, c in styles:
window.set_description(d)
window.set_attribute(a)
window.set_color_list(c)

is perfectly clear. Normally, I don't use single-letter variable names,
but in this particular case, the descriptive function names provide all
the context that's need to explain what they are.
 
L

Lawrence D'Oliveiro

In this case, I think I would do:

styles = [("normal", "image", MainWindow.ColorsNormalList),
("highlighted", "highlight", MainWindow.ColorsHighlightedList),
("selected", "select", MainWindow.ColorsSelectedList)]

for in description, attr, color_list in styles:
blah, blah, blah

And so you have managed to separate one set of looping conditions into two
parts. What is the significance of the name “styles� None at all. What
purpose does it serve? None, really. Does it ease the maintenance burden?
No, but by splitting your attention across two places, it actually adds to
it.

Here’s another example for you to chew on:

for \
name, reg, doindir \
in \
(
("R0", 0, True),
("R1", 1, True),
("R2", 2, True),
("R3", 3, True),
("R4", 4, True),
("R5", 5, True),
("R6", 6, True),
("SP", 6, True),
("R7", 7, False),
("PC", 7, False),
) \
:
...
#end for
 
R

Roy Smith

Lawrence D'Oliveiro said:
In this case, I think I would do:

styles = [("normal", "image", MainWindow.ColorsNormalList),
("highlighted", "highlight", MainWindow.ColorsHighlightedList),
("selected", "select", MainWindow.ColorsSelectedList)]

for in description, attr, color_list in styles:
blah, blah, blah

And so you have managed to separate one set of looping conditions into two
parts. What is the significance of the name “styles� None at all. What
purpose does it serve? None, really. Does it ease the maintenance burden?
No, but by splitting your attention across two places, it actually adds to
it.

I suppose readability is in the eye of the reader, but, yes, I agree
that I have split this into two parts. The parts are

1) The table of data

2) The looping construct

Where we seem to disagree is whether that makes things more or less
readable :) To me, it makes is more readable because it lets me
understand one chunk, then move on to understanding the next chunk.

You may disagree. That's OK.
 
R

Robert Kern

Instead, I'd do::

styles = [
("normal", "image", MainWindow.ColorsNormalList),
("highlighted", "highlight", MainWindow.ColorsHighlightedList),
("selected", "select", MainWindow.ColorsSelectedList)]

I'd go one step further:

styles = [
("normal", "image", MainWindow.ColorsNormalList),
("highlighted", "highlight", MainWindow.ColorsHighlightedList),
("selected", "select", MainWindow.ColorsSelectedList),
]

Keeping the []s separate from the items and using the trailing comma means that
I can easily add new items or reorder the items.

--
Robert Kern

"I have come to believe that the whole world is an enigma, a harmless enigma
that is made terrible by our own mad attempt to interpret it as though it had
an underlying truth."
-- Umberto Eco
 
L

Lawrence D'Oliveiro

Lawrence D'Oliveiro said:
In this case, I think I would do:

styles = [("normal", "image", MainWindow.ColorsNormalList),
("highlighted", "highlight",
MainWindow.ColorsHighlightedList),
("selected", "select", MainWindow.ColorsSelectedList)]

for in description, attr, color_list in styles:
blah, blah, blah

And so you have managed to separate one set of looping conditions into
two parts. What is the significance of the name “styles� None at all.
What purpose does it serve? None, really. Does it ease the maintenance
burden? No, but by splitting your attention across two places, it
actually adds to it.

I suppose readability is in the eye of the reader, but, yes, I agree
that I have split this into two parts. The parts are

1) The table of data

2) The looping construct

But the table of data makes no sense outside of the looping construct. That
table does nothing other than define the bounds of the loop. Without the
loop, it has no reason to exist. It makes no more sense than

styles = range(0, 100)
for i in styles :
...
Where we seem to disagree is whether that makes things more or less
readable :) To me, it makes is more readable because it lets me
understand one chunk, then move on to understanding the next chunk.

Which means you don’t understand the purpose of the code at all. Go look at
it in its entirety, and you’ll see what I mean.

<http://github.com/ldo/dvd_menu_animator>
 
C

Chris Rebert

And so you have managed to separate one set of looping conditions into two
parts. What is the significance of the name “desc_attr_colors_triples� None
at all. What purpose does it serve? None, really. Does it ease the
maintenance burden? No, but by splitting your attention across two places,
it actually adds to it.

If this is all your PEP-8 can achieve, then a pox on it.

Actually, my PEP 8 reference was in regards to the (imo, terrible)
UseOfCamelCaseForNonClasses (Python != C#), not the formatting of the
for-loop; hence the "In any case" qualification.

Cheers,
Chris
 
R

Roy Smith

I suppose readability is in the eye of the reader, but, yes, I agree
that I have split this into two parts. The parts are

1) The table of data

2) The looping construct

But the table of data makes no sense outside of the looping construct. That
table does nothing other than define the bounds of the loop.[/QUOTE]

I look at it the other way around. The data is the important part, and
the loop is just plumbing. Once I've got the data in an easy to manage
form, I can do lots of things with it. I could iterate over it (as
originally written) with a "for" loop. I could refactor the code to
pass the data to some function which processes it. I could refactor it
a different way and use map() to process the data.
Which means you don’t understand the purpose of the code at all. Go look at
it in its entirety, and you’ll see what I mean.

<http://github.com/ldo/dvd_menu_animator>

That URL takes me to a github page. Can you be more specific about
which file I should be looking at? I did take a look at one file,
overscan_margins.py. Here's a few stylistic comments.

1) I like the fact that the file starts out with a block comment
describing what it does, how to install it, and how to run it.
Documentation is A Good Thing, and most people don't do enough of it.

2) You have provided comments for each function, such as (lines 36-37):

def NewLayer(svg, LayerName) :
# adds a new layer to the SVG document and returns it.

This is good, but would be even more useful if it were turned into a
docstring, such as:

def NewLayer(svg, LayerName) :
"adds a new layer to the SVG document and returns it."

For the same amount of typing, you now have a piece of text which can be
retrieved by help() and processed in various useful ways with other
tools.

3) I find the deeply nested style you use very difficult to read. For
example, on lines 80-103. As I read this, here's how I mentally process
it:

"OK, here's a function call (minor stumble over the open paren not being
on the same line as the function name, but I can get past that). The
first argument is TheLayer. The second argument is whatever
inkex.addNS() returns. Umm..."

At that point, I can't scan quickly any more. It took me a while to
understand that the third argument was a dictionary. The nesting is
just too deep for me to continue to mentally hold the context of "I'm
looking at a function call and gathering up the arguments" while I drill
down through the dictionary structure to understand what it is.

This would be a lot easier to scan if it were written as:

inkex.etree.SubElement(TheLayer,
inkex.addNS("path", "svg"),
stuff)

where "stuff" is the big complicated dictionary, which is defined before
the call. Of course, "stuff" is not a useful name for it, but not being
familiar with the domain, I don't know what a useful name would be.

Hmmm, googling for inkex.etree.SubElement found me some Inkscape
documentation. Looks like "attribs" would be a good name for the
dictionary, since that's the name they use in the docs. In fact, if you
look at http://tinyurl.com/24wa3q8, you'll see they use exactly the
style I describe above (modulo whitespace).
 
N

Neil Cerutti

I would do the same, but fix the indentation. Making
indentation depend on the *length* of the previous line is
needlessly making a maintenance burden.

Instead, I'd do::

styles = [
("normal", "image", MainWindow.ColorsNormalList),
("highlighted", "highlight", MainWindow.ColorsHighlightedList),
("selected", "select", MainWindow.ColorsSelectedList)]

Agreed, except cute stuff like putting those three items in
columns is just as bad.

Code should be utilitarian rather than ornate, Shaker rather than
Victorian.
 
L

Lawrence D'Oliveiro

Chris said:
Actually, my PEP 8 reference was in regards to the (imo, terrible)
UseOfCamelCaseForNonClasses (Python != C#), not the formatting of the
for-loop; hence the "In any case" qualification.

Hmm ... OK, I might accept that particular criticism. I have to say it’s a
habit I picked up in my Mac programming days, that I find hard to shake. I
do sometimes deliberately use lowercase_with_underscores, but then I keep
LapsingBackAgain.

At least I prefer Bactrian to Dromedary. :)
 
L

Lawrence D'Oliveiro

That URL takes me to a github page. Can you be more specific about
which file I should be looking at?

The extract I previously quoted was from dvd_menu_animator.
2) You have provided comments for each function, such as (lines 36-37):

def NewLayer(svg, LayerName) :
# adds a new layer to the SVG document and returns it.

This is good, but would be even more useful if it were turned into a
docstring ...

I am ambivalent about the usefulness of docstrings. I find them mainly handy
in stuff imported from library modules, of which this is not one.
3) I find the deeply nested style you use very difficult to read. For
example, on lines 80-103. As I read this, here's how I mentally process
it:

"OK, here's a function call (minor stumble over the open paren not being
on the same line as the function name, but I can get past that). The
first argument is TheLayer. The second argument is whatever
inkex.addNS() returns. Umm..."

You’re looking at it wrong. It’s building a data structure to go into an SVG
file. Think of each piece of the expression as directly mapping to the
corresponding piece of the structure being built. The value being built for
the “d†attribute is just a sequence of control points for the path.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,744
Messages
2,569,484
Members
44,903
Latest member
orderPeak8CBDGummies

Latest Threads

Top