I am just testing out a few small functions for learning purposes:
def peaks(iterable):
# returns a list of int for those values in the iterable that are bigger
# than the value preceding and following them.
itr = iter(iterable)
peak =[]
curr = next(itr)
last = next(itr)
try:
while True:
first = curr
curr = last
last = next(itr)
if curr > first and curr > last:
peak.append(curr)
except:
pass
return peak
def compress(v_iterable,b_iterable):
#takes two iterables as parameters: it produces every value from the first iterable that
# has its equivalent position in the second iterable representing what Python would consider
# a True value. Terminate the iteration when either iterable terminates
mega_it = dict(zip(v_iterable, b_iterable))
for nxt in sorted(mega_it):
if mega_it[nxt]:
yield nxt
def skipper(iterable,n=0):
#akes one iterable and one int (whose default value is 0) as parameters: it produces values from
# the iterable, starting at the first one, skipping the number of values specified by the int
#(skipping 0 means producing every value from the iterable)
itr = iter(iterable)
yield next(itr)
while True:
for i in range(n):
skipped = next(itr)
yield next(itr)
I feel like my codes are lengthy for the kind of work it does. Is there a way to make my functions cleaner or smaller?
-
\$\begingroup\$ Could you please split this question into several questions? \$\endgroup\$200_success– 200_success2015年02月02日 20:55:16 +00:00Commented Feb 2, 2015 at 20:55
-
\$\begingroup\$ @200_success - I split the question into functions and classes. This one focuses on functions. \$\endgroup\$LucyBen– LucyBen2015年02月02日 23:20:24 +00:00Commented Feb 2, 2015 at 23:20
4 Answers 4
Your comments are good, but they should all be written as docstrings instead.
When working with iterators, itertools
and generator expressions are your friend. peaks()
works with three iterators, so zip()
is useful.
Since the function accepts an iterable, it seems fitting that it should be a generator rather than returning a list.
Python supports double-ended inequalities.
from itertools import tee
def peaks(iterable):
"""Generates values in the iterable that are greater than the value
preceding and following them."""
before, this, after = tee(iter(iterable), 3)
next(this)
next(after)
next(after)
for prev, curr, succ in zip(before, this, after):
if prev < curr > succ:
yield curr
If you wanted to keep the old interface, which returns a list, then a list comprehension would be a good way to write it.
def peaks(iterable):
"""Returns a list of values in the iterable that are greater than the
preceding and following value."""
xs, ys, zs = tee(iter(iterable), 3)
next(ys)
next(zs)
next(zs)
return [y for (x, y, z) in zip(xs, ys, zs) if x < y > z]
I find your compress()
weird, in that it sorts the results.
You can implement the function using just a generator expression.
def compress(values, incl_excl):
"""Keeping only values whose corresponding entry in incl_excl is true,
generates the sorted filtered list."""
yield from sorted(val for val, include in zip(values, incl_excl) if include)
In skipper()
, the two occurrences of yield next(itr)
should be combined.
def skipper(iterable, n=0):
"""Generator that yields the first element, then skips n values between
results, until the input is exhausted."""
iterable = iter(iterable)
while True:
yield next(iterable)
for _ in range(n):
next(iterable)
-
\$\begingroup\$ @ 200_success - sorry this might seem unrelated but when I make the changes to the function, and try to do
print(peaks([0,1,-1,3,8,4,3,5,4,3,8]))
it keeps giving me the memory location<generator object peaks at 0x0000000002935558>
instead of[1, 8, 5]
. \$\endgroup\$LucyBen– LucyBen2015年02月03日 02:09:25 +00:00Commented Feb 3, 2015 at 2:09 -
1\$\begingroup\$ That's what I meant by "it should be a generator rather than returning a list." If you want to display a list, then write
print(list(peaks([0,1,-1,...])))
. \$\endgroup\$200_success– 200_success2015年02月03日 02:13:39 +00:00Commented Feb 3, 2015 at 2:13 -
\$\begingroup\$ Thanks! Is it possible to make the function cleaner without making it a generator? I ask as the program should pass a pre-designed batch test which might just have
print(peaks([0,1,-1,3,8,4,3,5,4,3,8]))
written. \$\endgroup\$LucyBen– LucyBen2015年02月03日 02:41:26 +00:00Commented Feb 3, 2015 at 2:41 -
\$\begingroup\$ Sure. I'd use a list comprehension. \$\endgroup\$200_success– 200_success2015年02月03日 03:11:00 +00:00Commented Feb 3, 2015 at 3:11
skipper
's functionality is covered by itertools.islice
:
def skipper(iterable,n=0):
return itertools.islice(iterable, 0, None, n+1)
Just wanted to note that peaks
could've been implemented with just two iterators. Somewhere along these lines:
def peaks(iterable):
state = False
for previous, following in zip(iterable, iterable[1:]):
if state and previous > following:
yield previous
state = previous < following
Of course it's up to you to replace iterable[1:]
with an iterator.
Code presented by 200_success avoids the problem, but I think it should be pointed out that you are ignoring all errors. A catch-all is never a good idea, it hides problems. You should use:
except StopIteration:
pass
Variables that are assigned to but never used are not useful. Variable _
is often considered an anonymous variable, good for replacing i
. And skipped
can be just skipped (pun).
for i in range(n):
skipped = next(itr)
for _ in range(n):
next(itr)
Alternative implementation of skipper. I would recommend asserting (checking) if arguments are sane, negative n
would lead to weird unexpected results.
def skipper(iterable, n=0):
assert n >= 0
return (a for i,a in enumerate(iterable) if i%(n+1) == 0)