sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.43k stars 479 forks source link

Add more-itertools as a standard package #32100

Open mkoeppe opened 3 years ago

mkoeppe commented 3 years ago

https://pypi.org/project/more-itertools/

Then get rid of our homegrown iteration tools:

Useful:

CC: @tscrim @orlitzky @slel @yyyyx4

Component: refactoring

Issue created by migration from https://trac.sagemath.org/ticket/32100

orlitzky commented 2 years ago
comment:3

I'll change my mind about this if it lets us delete a good bit of custom code, but looking at the given examples:

mkoeppe commented 2 years ago
comment:5

Replying to @orlitzky:

I'll change my mind about this if it lets us delete a good bit of custom code

I agree, there needs to be a demonstrated substantial use before we add another dependency

slel commented 2 years ago
comment:6

One example is slicing a list into sublists of size "ncpu" to feed a parallel function.

For that, we can each time define a custom "list of lists" function, for instance

lol = lambda lst, sz: [lst[i:i+sz] for i in range(0, len(lst), sz)]

as found 17 times in sage/manifolds and sage/tensor (try git grep 'lol =').

We could instead use chunked, ichunked, sliced or isliced from more-itertools.

orlitzky commented 2 years ago
comment:7

lol = lambda lst, sz: [lst[i:i+sz] for i in range(0, len(lst), sz)]

Changing that line on its own won't eliminate much code, though. The pattern

lol = lambda lst, sz: [lst[i:i+sz] for i in range(0, len(lst), sz)]
ind_list = [ind for ind in other._comp] # only this line changes
ind_step = max(1, int(len(ind_list)/nproc/2))
local_list = lol(ind_list, ind_step)

is repeated over and over again. If the whole thing were factored out into a helper function, then we'd be left with changing

lol = lambda lst, sz: [lst[i:i+sz] for i in range(0, len(lst), sz)]

into

lol = lambda lst, sz: chunked(lst,sz)

which only saves us a few characters.

mkoeppe commented 2 years ago

Description changed:

--- 
+++ 
@@ -3,4 +3,7 @@
 Then get rid of our homegrown iteration tools:
 - from `sage.misc.misc`: `uniq`, `_stable_uniq`, `subsets`/`powerset`

+Useful:
+- [SequenceView](https://more-itertools.readthedocs.io/en/stable/api.html#more_itertools.SequenceView) - e.g. gives an idiom for taking the `len` of an iterable

+
mkoeppe commented 2 years ago

Description changed:

--- 
+++ 
@@ -6,4 +6,5 @@
 Useful:
 - [SequenceView](https://more-itertools.readthedocs.io/en/stable/api.html#more_itertools.SequenceView) - e.g. gives an idiom for taking the `len` of an iterable

+- `only`/`strictly_n`/`take` - [#34509 comment:52](https://github.com/sagemath/sage/issues/34509#comment:52)