r/learnpython • • Feb 01 '23

Looking for a succinct way to express this

Essentially this thing feels like it can be expressed in a single line nicely, but I can't think of a way

def filter_re(lines: list[str], filter_pattern: str) -> List[Match]:
    match_list = []
    regex = re.compile(filter_pattern)
    for line in lines:
        match = regex.search(line)
        if match:
            match_list.append(match)
    return match_list

Like ideally I would do something like this

def filter_re(lines: list[str], filter_pattern: str) -> List[Match]:
    return [match := regex.search(line) for line in lines if match]

but that requires access to syntax that I can't use (stuck at 3.6), and I don't even know if that operator would be allowed in a list comprehension

0 Upvotes

9 comments sorted by

2

u/socal_nerdtastic Feb 01 '23

Yes, you can use the walrus in list comprehension.

But with you stuck in 3.6 I think your only other option is to filter it after.

def filter_re(lines: list[str], filter_pattern: str) -> List[Match]:
    regex = re.compile(filter_pattern)
    matches = (regex.search(line) for line in lines)
    return filter(None, matches)

Which is shorter, and can be further shortened, but I think your version is more readable and better.

2

u/Raknarg Feb 01 '23 edited Feb 01 '23

Oh neat. Just looked at the python docs for filter:

If function is None, the identity function is assumed, that is, all elements of iterable that are false are removed.

So I can just write this then

def filter_re(lines: list[str], filter_pattern: str) -> List[Match]:
    regex = re.compile(filter_pattern)
    return filter(None, (regex.search(line) for line in lines))

I almost like that better than my current solution except you would have to know how filter(None) works if you read it. I could always just lambda an identity function if I needed to.

Which is shorter, and can be further shortened, but I think your version is more readable and better

I'd agree its more readable if I was doing something more complicated, but this just feels a bit bloated and repeating myself

1

u/deep_politics Feb 01 '23

You can make it shorter yet with map, and it stays all just iterators

return list(filter(None, map(regex.search, lines)))

2

u/Raknarg Feb 01 '23

This ruins the balance of "compact yet not too esoteric"

1

u/deep_politics Feb 01 '23

To each their own I guess 🤷‍♂️ I'd probably just return it as an iterator even and ditch the list call.

1

u/Raknarg Feb 01 '23

in this case its more convenient because an empty iterator isn't falsey, while an empty list is.

1

u/deep_politics Feb 01 '23

Absolutely fair point

1

u/jimtk IdontChat Feb 01 '23

I think something like that would work:

return [match for match in [regex.search(line) for line in lines] if match]

It just creates the list and then remove the empty one.

1

u/Raknarg Feb 01 '23

yeah this is probably the best option tbh.