Use native iterators instead of Lumino iterators

[afshin]

This PR refactors Lumino packages to switch to iterators and generators (Mozilla (MDN), TypeScript) instead of custom Lumino iterators. Fixes #333.

Iteration suggestions for Lumino users

These are some guiding principles that came up over the course of refactoring the way we handle iteration in Lumino. They are not hard and fast rules, rather they are intuitions that arose while implementing the changes in Lumino 2.

Use each(...) sparingly

Iterating through an iterable bar: Iterable<T> using native for...of, e.g., for (const foo of bar) {...} is almost always a better option than using each(bar, foo => ...). Some exceptions to this are:

• When you need the index of an item during iteration and you are not iterating through an array, which already provides the index as a second parameter to a function passed into .forEach(...)
• When you could use the array .forEach(...) method but you want to be able to terminate iteration early, which you cannot do natively, but can with each(...) by returning false

Nearly all invocations of each(...) have been removed in Lumino 2 (except for tests). See, for example, this commit.

Use [].forEach(...) sparingly

Now that we support native ES6 iteration, for (const value of someArray) {...} should be favored over someArray.forEach(...) because it will not require a context shift every time it invokes the function being applied.

Use [Symbol.iterator]() sparingly

Unless you need a handle on multiple iterators simultaneously (e.g., the way zip(...) is implemented in Lumino 2) or you need to hold on to multiple values of your iterable during iteration (e.g., the way we need both the first and the second value of an iterable to implement reduce(...) in Lumino 2), most of the time you can simply use for...of to iterate through any object that has a Symbol.iterator method without invoking that method.

In many places where the Lumino iter() utility function has been replaced in Lumino 2 it is not replaced with an invocation of the new Symbol.iterator method.

Use Array.from(...) sparingly

toArray(...) has been deprecated. You may be tempted to swap in Array.from(...) when you update your code. This will work, but if you simply need to iterate through an iterable, you can use for...of directly on the iterable object. This is more performant both in terms of CPU and memory than allocating and populating new Array instance before iteration.

If you need a snapshot of every item in your iterable as an array, then Array.from(...) is an appropriate replacement for toArray(...).

Public API changes

@lumino/algorithm

All of the iterator utilities have been changed to use native generators and iterators.

	export	name	note
❌	type	IterableOrArrayLike<T>	Switch to Iterable<T>
❌	interface	IIterable<T>	Switch to Iterable<T>
❌	interface	IIterator<T>	Switch to Iterator<T> / IterableIterator<T>
❌	function	iter<T>(...)	Switch to Symbol.iterator and function*
❌	function	iterFn<T>(...)	Switch to function*
❌	function	iterItems<T>(...)	We aren't using this function anywhere
❌	function	iterKeys<T>(...)	Switch to for ... in
❌	function	iterValues<T>(...)	Switch to for ... of
❌	class	ArrayIterator<T>	Switch to [][Symbol.iterator]()
❌	class	ChainIterator<T>	Previous implementation of chain<T>()
❌	class	EmptyIterator<T>	Previous implementation of empty<T>()
❌	class	EnumerateIterator<T>	Previous implementation of enumerate<T>()
❌	class	FilterIterator<T>	Previous implementation of filter<T>()
❌	class	FnIterator<T>	Switch to generators
❌	class	ItemIterator<T>	We aren't using this class anywhere
❌	class	KeyIterator	Switch to for ... in
❌	class	MapIterator<T>	Previous implementation of map<T>()
❌	class	RangeIterator<T>	Previous implementation of range()
❌	class	RetroIterator<T>	Previous implementation of retro<T>()
❌	class	StrideIterator<T>	Previous implementation of stride<T>()
❌	class	TakeIterator<T>	Previous implementation of take<T>()
❌	class	ValueIterator<T>	Switch to for ... of
❌	class	ZipIterator<T>	Previous implementation of zip<T>()
✅	function	chain<T>(...)	Reimplement with native types
✅	function	each<T>(...)	Reimplement with native types
✅	function	empty<T>(...)	Reimplement with native types
✅	function	enumerate<T>(...)	Reimplement with native types
✅	function	every<T>(...)	Reimplement with native types
✅	function	filter<T>(...)	Reimplement with native types
✅	function	find<T>(...)	Reimplement with native types
✅	function	findIndex<T>(...)	Reimplement with native types
✅	function	map<T>(...)	Reimplement with native types
✅	function	max<T>(...)	Reimplement with native types
✅	function	min<T>(...)	Reimplement with native types
✅	function	minmax<T>(...)	Support native types
✅	function	reduce<T>(...)	Support native types
✅	function	range(...)	Reimplement with native types
✅	function	retro<T>(...)	Reimplement with native types
✅	function	some<T>(...)	Reimplement with native types
✅	function	stride<T>(...)	Reimplement with native types
✅	function	take<T>(...)	Reimplement with native types
☑️	function	toArray<T>(...)	@deprecated, use Array.from(...) or for ... of
✅	function	toObject(...)	Reimplement with native types
✅	function	topologicSort<T>(...)	Support native types
✅	function	zip<T>(...)	Reimplement with native types

@lumino/collections

LinkedList has been updated to accept native iterables and return native iterators.

	export	name	note
❌	class	LinkedList.ForwardValueIterator	Switch to LinkedList#[Symbol.iterator]
❌	class	LinkedList.RetroValueIterator	Previous implementation of LinkedList#retro()
❌	class	LinkedList.ForwardNodeIterator	Previous implementation of LinkedList#nodes()
❌	class	LinkedList.RetroNodeIterator	Previous implementation of LinkedList#retroNodes()
❌	method	LinkedList#iter()	Switch to LinkedList#[Symbol.iterator]
✅	function	LinkedList.from<T>(...)	Accept Iterable<T>
✅	method	LinkedList#assign(...)	Accept Iterable<T>
✅	method	LinkedList#nodes()	Return IterableIterator<LinkedList.INode<T>>
✅	method	LinkedList#retro()	Return IterableIterator<T>
✅	method	LinkedList#retroNodes()	Return IterableIterator<LinkedList.INode<T>>

@lumino/datagrid

DataGrid selections are now native iterators.

	export	name	note
✅	method	BasicSelectionModel#selections()	Return IterableIterator<SelectionModel.Selection>
✅	method	SelectionModel#selections()	Return IterableIterator<SelectionModel.Selection>

@lumino/disposable

Helper functions for DisposableSet and ObservableDisposableSet have been udpated.

	export	name	note
✅	function	DisposableSet.from(...)	Accept Iterable<IDisposable>
✅	function	ObservableDisposableSet.from(...)	Accept Iterable<IDisposable>

@lumino/widgets

Layout and its sub-classes now use native iterators, e.g. implements Iterable<Widget>.

	export	name	note
❌	method	DockLayout#iter()	Switch to DockLayout#[Symbol.iterator]
❌	method	GridLayout#iter()	Switch to GridLayout#[Symbol.iterator]
❌	method	Layout#iter()	Switch to Layout#[Symbol.iterator]
❌	method	PanelLayout#iter()	Switch to PanelLayout#[Symbol.iterator]
❌	method	SingletonLayout#iter()	Switch to SingletonLayout#[Symbol.iterator]
✅	method	DockLayout#handles()	Return IterableIterator<HTMLDivElement>
✅	method	DockLayout#selectedWidgets()	Return IterableIterator<Widget>
✅	method	DockLayout#tabBars()	Return IterableIterator<TabBar<Widget>>
✅	method	DockLayout#widgets()	Return IterableIterator<Widget>
✅	method	DockPanel#handles()	Return IterableIterator<HTMLDivElement>
✅	method	DockPanel#selectedWidgets()	Return IterableIterator<Widget>
✅	method	DockPanel#tabBars()	Return IterableIterator<TabBar<Widget>>
✅	method	DockPanel#widgets()	Return IterableIterator<Widget>
✅	method	Widget#children()	Return IterableIterator<Widget>

[afshin]

cc: @jasongrout @vidartf @ibdafna @mwcraig — apropos our conversation about Lumino 2's impact on @jupyter-widgets/* packages on the Jupyter Widgets weekly call.

[blink1073]

Nice work, thank you!

[fcollonval]

thanks a lot for this @afshin

Would you mind adding a migration.rst file in the documentation that include your very useful advices from the this PR description.

[afshin]

I added migration.md (because the docs apparently can be .rst or .md) but I'm still not sure the docs are building/working properly. I'd propose leaving that to fix in:

• Author Lumino 1 → Lumino 2 migration guide #340,
• Update RTD to include multiple versions of Lumino #336

[vidartf]

Good work here! I tried my best to review the code in the actual algorithm package vs behavior changes, and made some comments w.r.t that. Also added some nitpicks since I was already looking 😄

As a maintainer of a few library packages dependent on Lumino, my main question for Lumino v2 will be "how hard will it be for my code-base to support both v1 and v2 during the transition period?". The fewer conditionals I need on version or contorted imports that will be needed, the better managed the upgrade will be. In that regards, I think there might be value in keeping the toArray and toObject functions around as deprecated convenience functions. AFAICT, there should be little maintenance cost in keeping them for a cycle, while it should bring high benefit to anyone that wants that ^1 || ^2 dependency.

[afshin]

As a maintainer of a few library packages dependent on Lumino, my main question for Lumino v2 will be "how hard will it be for my code-base to support both v1 and v2 during the transition period?". The fewer conditionals I need on version or contorted imports that will be needed, the better managed the upgrade will be. In that regards, I think there might be value in keeping the toArray and toObject functions around as deprecated convenience functions. AFAICT, there should be little maintenance cost in keeping them for a cycle, while it should bring high benefit to anyone that wants that ^1 || ^2 dependency.

Because the types have changed, even a convenience toArray and toObject won't stop you from having to rewrite code. I strongly advocate for removing these, but I'm prepared to be outvoted on this 🙂

[jasongrout]

Because the types have changed, even a convenience toArray and toObject won't stop you from having to rewrite code. I strongly advocate for removing these, but I'm prepared to be outvoted on this 🙂

If I have code toArray(luminoiterator), how would I need to rewrite code? Before luminoiterator was a lumino iterator, now it is a js iterator, but either way I get an array in the end, without having to change my code, right?

[afshin]

If I have code toArray(luminoiterator), how would I need to rewrite code? Before luminoiterator was a lumino iterator, now it is a js iterator, but either way I get an array in the end, without having to change my code, right?

If your luminoiterator is now a native iterable by virtue of something upstream having changed its types, then I would have some suggestions for how you should change your code:

1. Do you really need for it to be an array? In many cases, you should probably use for ... of
2. If you truly need it to be an array, use Array.from(luminoiterator) (with the understanding that luminoiterator is no longer a Lumino iterator)

[jasongrout]

for how you should change your code:

Right - I thought what @vidartf was proposing was deprecated functions that allowed you to do ^1||^2 without changing code, without too much burden on lumino maintainers (i.e., keep toArray since it is used often). I'm +1 on easing the transition with a deprecation cycle for common functions.

[afshin]

@vidartf, @jasongrout, @davidbrochart, two things:

1. I reimplemented toObject(...) even though we weren't using it because it's pretty straightforward and doesn't hurt.
2. I re-added toArray with a @deprecated tag.

[blink1073]

Let's merge and...iterate.