0

I have a set I want to update if there is match in another set. Else I want to append to a list strings of error messages if there is no match. I referenced if/else in a list comprehension to write my code.

Here is what I wrote:

logstocrunch_set=dirlogs_set.difference(dblogs_set)
pattern = re.compile(r"\d*F[IR]P",re.IGNORECASE)  #to find register values
logstocrunch_finset = set()
errorlist = []
logstocrunch_finset.update([x for x if pattern.search(x) else errorlist.append(f'{x} is not proper name') for x in logstocrunch_set])

However, when I run this, I get the error invalid syntax with the arror pointed at my if statement.

So why is this happening?

Barmar
  • 741,623
  • 53
  • 500
  • 612
edo101
  • 629
  • 6
  • 17
  • Please see [How to ask a good question](https://stackoverflow.com/help/how-to-ask). Always [Provide a Minimal, Reproducible Example (e.g. code, data, errors, current output, expected output), as text](https://stackoverflow.com/help/minimal-reproducible-example). _A proper question **MUST** provide **ALL** of the information necessary in order for a proper answer to be given._ – Trenton McKinney Aug 10 '20 at 20:57
  • @TrentonMcKinney I provided sample code... and told you what the error is in the body.. what else are you talking about? I can't post my entire code here because people will complain I have too much code. What exactly else do I need to clarify? – edo101 Aug 10 '20 at 21:00
  • 1
    You could try the pattern recommended in the "not a duplicate" answer, `f(x) if condition...` instead of the `x for x if condition` that you have. – Dennis Sparrow Aug 10 '20 at 21:02
  • You should probably start with the fact that your list comprehension starts with “x for x if ...”. x for x in what? You need something to iterate over with a for statement – Ethan Aug 10 '20 at 21:03
  • You have `for x` *twice*, it's *not* properly formatted. Also you shouldn't use a list comprehension for a side effect, it seems like you're trying to write a *partition* – jonrsharpe Aug 10 '20 at 21:04
  • 2
    You can't have `else` in the `if` condition after `for`. – Barmar Aug 10 '20 at 21:06
  • 1
    It would work if you remove that extra "for x": `[x if pattern.search(x) else errorlist.append(f'{x} is not proper name') for x in logstocrunch_set]`. The general pattern you want here is `x if thing1 else thing2 for x in thing`. BTW, this will add `None` to the set as that's what's returned by `errorlist.append`. – tdelaney Aug 10 '20 at 21:10
  • You *could* do e.g. `[foo for foo in bar if baz(foo) or errors.append("whoops")]`, but... don't. – jonrsharpe Aug 10 '20 at 21:18
  • 1
    You **really** shouldn't use side-effects in a list comprehension – juanpa.arrivillaga Aug 10 '20 at 21:20

2 Answers2

2

The syntax of a list comprehension with a condition is:

[<value> for <variable> in <iterable> if <condition>]

if <condition> goes after the iterable, not before it.

Also, you can't have an else clause there. It's not a conditional expression that returns different values, it's just used to filter the values in the iterator, so else makes no sense.

You seem to be confusing it with a conditional expression in the <value> part, which allows you to specify different values to be returned in the resulting list depending on a condition. That's just an ordinary conditional expression, not specific to list comprehensions.

You shouldn't use a list comprehension if you want to update multiple targets. Use an ordinary loop.

logstocrunch_finset = set()
errorlist = []
for x in logstocrunch_set:
    if pattern.search(x):
        logtocrunch_finset.add(x)
    else:
        errorlist.append(f'{x} is not proper name')
Barmar
  • 741,623
  • 53
  • 500
  • 612
  • Thanks, I wonder if using two comprehensions would be faster than one normal for loop? @Barmar – edo101 Aug 10 '20 at 21:39
  • I can't imagine any way that it would be. It does the same number of calls to `append()` and `add()`, but does twice the number of loops and `if` tests. – Barmar Aug 10 '20 at 21:41
2

A list comprehension is a way of creating a single list. A basic conditional one must be in the format:

[ expression for item in iterable if condition ]

You can't (easily) update two objects with one comprehension. Also, there's not a lot of point declaring logstocrunch_finset and errorlist and then populating them. Instead, how about something like:

pattern = re.compile(r"\d*F[IR]P", re.IGNORECASE)
logstocrunch_finset = {x for x in logstocrunch_set if pattern.search(x)}
errorlist = [f'{x} is not proper name' for x in logstocrunch_set.difference(logstocrunch_finset)]

UPDATE BELOW - Performance comparison with for loop

As @Barmar suggested, I benchmarked our two solutions. There's not a lot in it. The two comprehensions seem to handle a larger input set better. Changing the ratio of valid to invalid data didn't seem to make much difference.

import re

range_limit = 10
logstocrunch_set = set(
    [f'{i}FRP' for i in range(range_limit)] + 
    [f'longer_{i}frp_lower' for i in range(range_limit)] + 
    ['not valid', 'something else']
)
pattern = re.compile(r"\d*F[IR]P",re.IGNORECASE)

%%timeit -n 100000 -r 20

logstocrunch_finset = set()
errorlist = []
for x in logstocrunch_set:
    if pattern.search(x):
        logstocrunch_finset.add(x)
    else:
        errorlist.append(f'{x} is not proper name')
  • range_limit = 10 | 9.53 µs ± 34.2 ns per loop (mean ± std. dev. of 20 runs, 100000 loops each)
  • range_limit = 50 | 45.5 µs ± 699 ns per loop (mean ± std. dev. of 20 runs, 100000 loops each)
  • range_limit = 100 | 89.4 µs ± 1.2 µs per loop (mean ± std. dev. of 10 runs, 100000 loops each)
%%timeit -n 100000 -r 20

logstocrunch_finset = {x for x in logstocrunch_set if pattern.search(x)}
errorlist = [f'{x} is not proper name' for x in logstocrunch_set.difference(logstocrunch_finset)]
  • range_limit = 10 | 9.58 µs ± 14.1 ns per loop (mean ± std. dev. of 20 runs, 100000 loops each)
  • range_limit = 50 | 42.2 µs ± 24.7 ns per loop (mean ± std. dev. of 20 runs, 100000 loops each)
  • range_limit = 100 | 82.2 µs ± 491 ns per loop (mean ± std. dev. of 10 runs, 100000 loops each)
Jules SJ
  • 430
  • 2
  • 8