Home     |     .Net Programming    |     cSharp Home    |     Sql Server Home    |     Javascript / Client Side Development     |     Ajax Programming

Ruby on Rails Development     |     Perl Programming     |     C Programming Language     |     C++ Programming     |     IT Jobs

Python Programming Language     |     Laptop Suggestions?    |     TCL Scripting     |     Fortran Programming     |     Scheme Programming Language


 
 
Cervo Technologies
The Right Source to Outsource

MS Dynamics CRM 3.0

Python Programming Language

Good Python style?


Hi,

I found the following quite cryptic code, which basically reads the
first column of some_file into a set.
In Python I am used to seeing much more verbose/explicit code. However,
the example below _may_ actually be faster than the usual "for line in ..."
Do you consider this code good Python style? Or would you recommend to
refrain from such complex single-line code??

Thanks!
Andreas

inp = resource(some_file)
# read first entries of all non-empty lines into a set
some_set = frozenset([line.split()[0] for line in \
          filter(None, [ln.strip() for ln in inp])])

On May 31, 2007, at 2:59 AM, Andreas Beyer wrote:

I don't know about style, but I find it much harder to read than this:

some_set = frozenset(line.split()[0]
                      for line in inp
                      if len(line.strip()))

...which I *think* is equivalent, but less complicated.

regards,
Michael
---
(do (or (do (not '(there is no try))) ()))

It's a little complex, but not excessively so. Any more, and it would
probably be too complex.

It would probably be easier to read with more readable names and a few
comments:

some_set = frozenset(
           # ... from a list comp of the first word in each line
           [line.split()[0] for line in
           # ... each line has leading and trailing white space
           # filtered out, and blanks are skipped
           filter(None, # strip whitespace and filter out blank lines
           [line.strip() for line in input_lines]
           )])

Splitting it into multiple lines is self-documenting:

blankless_lines = filter(None, [line.strip() for line in input_lines])
first_words = [line.split()[0] for line in blankless_words]
some_set = frozenset(first_words)

As for which is faster, I doubt that there will be much difference, but I
expect the first version will be a smidgen fastest. However, I'm too lazy
to time it myself, so I'll just point you at the timeit module.

--
Steven.

On May 31, 3:59 am, Andreas Beyer <m@a-beyer.de> wrote:

I think it would be more readable if you would take the filter out of
the list comprehension, and the list comprehension out of the set.

inp = resource(some_file)
stripped_lines = (ln.strip() for ln in inp)
splitted_lines = (line.split()[0] for line in stripped_lines if line)
some_set = frozenset(splitted_lines)

Steven D'Aprano wrote:
> It would probably be easier to read with more readable names and a few
> comments:

[...]

> Splitting it into multiple lines is self-documenting:

> blankless_lines = filter(None, [line.strip() for line in input_lines])
> first_words = [line.split()[0] for line in blankless_words]
> some_set = frozenset(first_words)

Re-writing code so that it is self-documenting is almost always a better
approach. Premature optimization is the root of all evil.
--
Michael Hoffman

Sparse is better than dense, and this code is far from the fastest one
could write -- it builds useless lists comprehensions where genexps
would do, it splits off all whitespace-separated words (rather than just
the first one) then tosses all but the first away, etc.

frozenset(first_word_of_line
     for line in input_file
     for first_word_of_line in line.split(None, 1)[:1]
     )

does not sacrifice any speed (on the contrary), yet achieves somewhat
better clarity by clearer variable names, reasonable use of whitespace
for formatting, and avoidance of redundant processing.

If this set didn't have to be frozen, building it in a normal for loop
(with a .add call in the nested loop) would no doubt be just as good in
terms of both clarity and performance; however, frozen sets (like
tuples) don't lend themselves to such incremental building "in place"
(you'd have to build the mutable version, then "cast" it at the end; the
alternative of making a new frozenset each time through the loop is
clearly unpleasant), so I can see why one would want to avoid that.

A good, readable, and only slightly slower alternative is to write a
named generator:

def first_words(input_file):
    for line in input_file:
        first_word_if_any = line.split(None, 1)
        if first_word_if_any:
            yield first_word_if_any[0]

ff = frozenset(first_words(input_file))

A full-fledged generator gives you more formatting choices, an extra
name to help, and the possibility of naming intermediate variables.
Whether it's worth it in this case is moot -- personally I find the
"slice then for-loop" idiom (which I used in the genexp) just as clear
as the "test then index" one (which I used in first_words) to express
the underlying notion "give me the first item if any, but just proceed
without giving anything if the list is empty"; but I can imagine
somebody else deciding that the test/index idiom is a more direct
expression of that notion.  You could use it in a genexp, too, of
course:

frozenset(first_word_if_any[0]
    for line in input_file
    for first_word_if_any in [line.split(None, 1)]
    if first_word_if_any
    )

but this requires the "for x in [y]" trick to simulate the "x=y"
assigment of an intermediate variable in a genexp or LC, which is never
an elegant approach, so I wouldn't recommend it.

Alex

Add to del.icio.us | Digg this | Stumble it | Powered by Megasolutions Inc