Python: Help with replacing list comprehension with generators [closed]

Question

Closed. This question is off-topic. It is not currently accepting answers.

---

Want to improve this question? Update the question so it's on-topic for Software Engineering Stack Exchange.

Closed 6 months ago.

Improve this question

I have written the following code which takes a coord_list of points in a 2D coordinate system, a center and a radius and returns a list of all points from the list having distance at most radius from center.

def points_in_rad(coord_list, center, radius):
    distances = [distance(coord, center) for coord in coord_list]
    result = [coord_list[i] for i in range(len(coord_list)) if distances[i] <= radius]
    return result

I am utilising here a function distance computing the distance between two points. The code works well but I am expecting coord_list to contain many points, say 10k or more. Performing two list comprehensions is thus quite slow and requires some memory and I would like to speed up this function and at the same time reduce the memory requirement. My idea was to use generators. For example, I could just write distances = (distance(coord, center) for coord in coord_list) but I'm struggling with converting the second list comprehension into a generator also as this requires looking up distances at a specific index.

Any ideas on how this can be done or other improvements to the code?

Answer 1

Well, you don't need distances at all. You could simply do:

def points_in_rad(coord_list, center, radius):
    return (coord for coord in coord_list if distance(coord, center) <= radius)

... although I think it would be more readable like this:

def points_in_rad(coord_list, center, radius):
    for coord in coord_list:
        if distance(coord, center) <= radius:
            yield coord

In general, if you ever find yourself doing for i in range(len(something)) in Python, you're missing something.

Also, the function name is ambiguous. At first I thought it meant points_in_radians, because that's what "rad" usually stands for in my experience. I don't think it's a good idea to save 3 characters that way.