Some suggestions to improve `bifurcate`, `find_last_index` and more

[veronicaguo]

Hi, here are a few things I noticed that can probably be improved. I'm happy to open a PR for these changes if people think they can add some value.

1. bifurcate: use zip() instead of enumerate()

   Current implementation:

   def bifurcate(lst, filter):
     return [
       [x for i, x in enumerate(lst) if filter[i] == True],
       [x for i, x in enumerate(lst) if filter[i] == False]
     ]

   Can be refactored into:

   def bifurcate(lst, filter):
     return [
       [x for x, flag in zip(lst, filter) if flag],
       [x for x, flag in zip(lst, filter) if not flag]
     ]

2. find_last_index: use built-in lst.index() to find index

   Current implementation:

   def find_last_index(lst, fn):
     return len(lst) - 1 - next(i for i, x in enumerate(lst[::-1]) if fn(x))

   Can be refactored into:

   def find_last_index(lst, fn):
     return next(lst.index(x) for x in lst[::-1] if fn(x))

3. chunk and geometric_progression

   • refactor range(0, val) to range(val).

[Trinityyi]

These seem like valid suggestions. You can PR them and we'll check and merge them as soon as we can.

[gurukiran07]

1 and 3 are good suggestions but 2nd one is logically wrong. Let's try a test case which has duplicates:

lst = [1, 2, 2, 1]
fn = lambda x: x%2==1
# current implementation output
# 4
# Your suggestion gives
# 0

list.index gives the index of the first occurrence. And talking about performance, the current implementation is better lst[::-1] takes O(N) and since the current implementation is using generator comprehension which evaluates lazily(computes values only on demand) and just spits out the index only once, len(lst) is O(1) in Python.

What you are doing does is lst[::-1] takes O(n) and does lst.index which O(N), making it O(2N), though this doesn't matter much but why iterate thrice(1 for reversing, 2 iterating through the reversed list, 3 finding lst.index). Since you too are using generator exp 2 iterating through the reversed list only happens lazily though. Performance-wise current implementation is a little better.

@Trinityyi @veronicaguo

[gurukiran07]

I have suggestion here,

def find_last_index(lst, fn):
    return next(i for i in range(len(lst) - 1, -1, -1) if fn(lst[i]))

This implementation doesn't need to reverse the list.

@Trinityyi

[JahnaviChitta]

Python is simple language with many built-in functions, the only thing is we have know their proper, efficient implementation. I think this repository is completely doing that job. I am greatly interested in contributing my part to this issue, I would be glad if you assign me this.

@Trinityyi @veronicaguo

[veronicaguo]

Thanks @gurukiran07 ! I think your counter example is valid. I'll open a PR with 1 and 3 for now.