5
\$\begingroup\$

I was wondering if there is a better way to solve this issue.

I have a list like this:

imp_list=["aa","daa","ab","as","aem",
      "aum","aw","aa","acm","at",
      "ar","aa_imp","daa_imp","ab_imp",
      "as_imp"]

I want to select all the strings that have the _imp suffix, plus all the the strings that have no _imp partner.

That's because aa_imp it's just a modified version of aa, and for me, in a large sense, it's a replica. So I created this function:

def imputed_handler(my_list): 
  imp=[x for x in my_list if "imp" in x]

  cleaned_imp=set(map(lambda x: x.replace("_imp",""),imp))

  not_imp=[x for x in my_list if "imp" not in x]
  set_list=set(my_list)
  no_replica=list(set_list-cleaned_imp)
  print(no_replica)
  return no_replica

Running the code as described above

test=imputed_handler(imp_list)

I get the following output:

['at', 'acm', 'aw', 'ar', 'daa_imp', 'aem', 'as_imp', 'ab_imp', 'aa_imp', 'aum']

Do better solutions exist? Thanks for your time, and let me know if something is not clear :)

\$\endgroup\$
4
  • 2
    \$\begingroup\$ "Do better solutions exist?" - Yes, for example the one that doesn't compute not_imp and then never uses it :-P \$\endgroup\$ – Kelly Bundy Feb 9 at 16:00
  • \$\begingroup\$ @KellyBundy lol you are right and you make my day :) \$\endgroup\$ – Andrea Ciufo Feb 9 at 16:10
  • 1
    \$\begingroup\$ What's with the function name, would you like to have it reviewed? Or is this a no-touch constraint? \$\endgroup\$ – hc_dev Feb 9 at 18:51
  • \$\begingroup\$ @hc_dev yep it can be reviewed! I am self-taught so if there is something wrong I really appreciate it! <3 \$\endgroup\$ – Andrea Ciufo Feb 10 at 10:49
10
\$\begingroup\$

First off, since you're looking for strings that end with _imp, x.endswith("_imp") is probably more reliable than "imp" in x.

Second, since we only want to remove _imp when it's a suffix, and since we know how long it is, we can use string slicing to remove the last 4 characters. That means we don't accidentally turn x_impabc_imp into xabc instead of x_impabc or something. Assuming that's not what we want to do. Maybe it is, maybe it isn't, maybe we'll never get an input like that so it won't matter either way, I have no idea.

I'm also thinking it might be nice to pass the suffix as a parameter. Maybe it might change at some point in the future. Maybe another part of the program uses a different suffix in a similar way. Doesn't hurt to be prepared.

In general, though, your approach is good. You find the _imp ones, you figure out what they are replacing, you remove the ones that have been replaced.

The neat thing is, you can do the first two steps all at once in a single pass through the input list. For example, you can do this:

def imputed_handler(my_list, suffix="_imp"):
    my_set = set(my_list)
    cleaned_imp = set(item[:-len(suffix)] for item in my_set if item.endswith(suffix))

    return list(my_set - cleaned_imp)

Though a loop might be clearer at that point:

def imputed_handler(my_list, suffix="_imp"):
    my_set = set(my_list)

    for item in my_list:
        if item.endswith(suffix):
            my_set.discard(item[:-len(suffix)])

    return list(my_set)
\$\endgroup\$
3
  • 5
    \$\begingroup\$ Since Python 3.9 you can also use item.removesuffix(suffix). \$\endgroup\$ – Kelly Bundy Feb 9 at 18:01
  • 1
    \$\begingroup\$ @KellyBundy Oh, neat. I didn't find that since I'm still on 3.8.5 for some reason, but that's good to know. \$\endgroup\$ – Sara J Feb 9 at 18:03
  • \$\begingroup\$ If I recall correctly, the line cleaned_imp = set(item[:-len(suffix)] for item in my_set if item.endswith(suffix)) will call len(suffix) and calculate -len(suffix) for every item. So it's usually better to calculate it once and use it as a variable inside the generator expression. \$\endgroup\$ – riskypenguin Apr 8 at 12:27
9
\$\begingroup\$

PEP 8

The Style Guide for Python Code lists several conventions Python programs should follow. Things like:

  • spaces around binary operators (imp = [...] and set_list = set(my_list)).
  • spaces after comma (eg, ""), imp))

Side Effects

You are returning a result from imputed_hander; shouldn't print from inside it.

Type Hints and Doc Strings

What is def imputed_handler(my_list)? What does it do? What is my_list a list of? What is returned?

from typing import List

def imputed_handler(my_list: List[str]) -> List[str]: 
    """
    Filter a list of strings, removing `"xyz"` if `"xyz_imp"` is found.
    Return the filtered list.
    """

Now we have a (poor) description, and can see the argument & return types.

Improved code

As mentioned in Sara J's answer, .endswith() is preferable to simply checking if the search string is contained anywhere inside the original.

Converting the list into a set, and then back into a list ruins the original lists order. Here is a possible improvement on Sara's solution:

from typing import List

def imputed_handler(my_list: List[str], suffix: str = "_imp") -> List[str]:
    """
    A good description here.
    """

    unwanted = {item.removesuffix(suffix) for item in my_list if item.endswith(suffix)}
    return [item for item in my_list if item not in unwanted]

A set is only constructed from the prefix of items which end in _imp. This should result in a smaller memory footprint, that set(my_list) - cleaned_imp. Since it is a set, the in operator is \$O(1)\$, filtering the list is fast.

\$\endgroup\$
2
  • 2
    \$\begingroup\$ Note, typing.List is now deprecated you can just use list[str] in Python 3.9+ or import annotations from PEP 563 - "Importing those from typing is deprecated. Due to PEP 563 and the intention to minimize the runtime impact of typing, this deprecation will not generate DeprecationWarnings." \$\endgroup\$ – Peilonrayz Feb 10 at 5:12
  • \$\begingroup\$ One more possible tweak is instead of taking "foo_imp" and generating "foo" to discard, is to ask for each "foo" whether "foo_imp" is in the set. my_set = set(my_list); return [s for s in my_list if s + suffix not in my_set]. Need to be careful whether it's equivalent for chains such as imputed_handler(["aa", "aa_imp", "aa_imp_imp"]) but I think it is... This also matches your description "removing "xyz" if "xyz_imp" is found" which is the clearest definition I can think of. \$\endgroup\$ – Beni Cherniavsky-Paskin Feb 10 at 17:47

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.