MAINT Use list comprehension #12883
MAINT Use list comprehension #12883
Conversation
lib/mpl_toolkits/mplot3d/art3d.py
Outdated
| return segments, codes_list | ||
| segments, codes_list = zip( | ||
| *[path_to_3d_segment_with_codes(path, pathz, zdir) | ||
| for path, pathz in zip(paths, zs)]) |
There was a problem hiding this comment.
Related to #11577, this should be faster,
In [4]: sequence = list(zip(range(1000), range(1000)))
In [5]: %%timeit
...: x = []
...: y = []
...: for x_el, y_el in sequence:
...: x.append(x_el)
...: y.append(y_el)
...:
135 µs ± 498 ns per loop (mean ± std. dev. of 7 runs, 10000 loops each)
In [6]: %%timeit
...: x, y = zip(*sequence)
...: x = list(x)
...: y = list(y)
...:
...:
49.2 µs ± 234 ns per loop (mean ± std. dev. of 7 runs, 10000 loops each)
There was a problem hiding this comment.
This is indeed a good performance improvement! However, it gets hard to read because there's too much going on. I propose to construct the list beforehand and give it a name. This should be equally fast.
segments_and_codes_list = [
path_to_3d_segment_with_codes(path, pathz, zdir)
for path, pathz in zip(paths, zs)
]
segments, codes = zip(*segments_and_codes_list)
return list(segments), list(codes)
lib/matplotlib/legend_handler.py
Outdated
| l = Line2D([thisx, thisx], [bottom, thisy]) | ||
| leg_stemlines.append(l) | ||
| leg_stemlines = [Line2D([thisx, thisx], [bottom, thisy]) | ||
| for thisx, thisy in zip(xdata_marker, ydata)] |
There was a problem hiding this comment.
The "this" prefix doesn't add any information and just makes it harder to read. While you're at it, can you please change this to for x, y in ...? That would further improve the code.
lib/mpl_toolkits/mplot3d/art3d.py
Outdated
| return segments, codes_list | ||
| segments, codes_list = zip( | ||
| *[path_to_3d_segment_with_codes(path, pathz, zdir) | ||
| for path, pathz in zip(paths, zs)]) |
There was a problem hiding this comment.
This is indeed a good performance improvement! However, it gets hard to read because there's too much going on. I propose to construct the list beforehand and give it a name. This should be equally fast.
segments_and_codes_list = [
path_to_3d_segment_with_codes(path, pathz, zdir)
for path, pathz in zip(paths, zs)
]
segments, codes = zip(*segments_and_codes_list)
return list(segments), list(codes)
Co-Authored-By: rth <rth.yurchak@pm.me>
|
Thanks for the review @timhoffm ! Addressed your comments.. |
|
Thanks for you contribution, hoping to see you around again! |
| seg.append((x, y, z)) | ||
| codes.append(code) | ||
| seg_codes = [((x, y, z), code) for ((x, y), code), z in zip(pathsegs, zs)] | ||
| seg, codes = zip(*seg_codes) |
There was a problem hiding this comment.
This should be protected by an empty-check, because it'll fail if pathsegs or zs is empty. With the previous implementation, you got back empty lists.
| return segments, codes_list | ||
| segments_codes = [path_to_3d_segment_with_codes(path, pathz, zdir) | ||
| for path, pathz in zip(paths, zs)] | ||
| segments, codes = zip(*segments_codes) |
There was a problem hiding this comment.
This also needs to be protected by an empty-check.
|
Thanks @WeatherGod , I forgot about the same comment by you from #11577 (comment) indeed. Will do a follow up PR with a fix. |
|
Made a PR to fix the added the zip unpacking issue in #12927 |
This adds a few list comprehensions in places where it doesn't hurt readability, which should be somewhat faster and less verbose.