Use FromIterator trait rather than custom method #648
Conversation
|
It definitely looks like the right direction @max-sixty! Adding An important point to keep in mind when working on this PR: make sure to update ndarray for NumPy users to reflect the changes you are introducing! |
|
OK great; I updated the code throughout. Apart from the docs, what else? Do we need to wait for a minor version to merge the |
There was a problem hiding this comment.
I've added some minor comments; everything else looks good to me.
I'm only now understanding how the rust import system works; I initially didn't realize you'd need an additional
usestatement each time to usefrom_iter.
This is because from_iter is provided by the FromIterator trait, and FromIterator is not included in the standard Rust prelude (so you have to explicitly use it). In contrast, collect is provided by the Iterator trait, which is included in the standard Rust prelude.
Should we be using
.collect()in the examples?
I think that's a bit nicer than adding use std::iter::FromIterator, but either way is fine.
Or adding the
std::iter::FromIterator;in thendarraystandard prelude?
I don't think we should do that because IMO that would be somewhat surprising.
Do we need to wait for a minor version to merge the
from_iterremoval?
We should include this as part of 0.13.0, which is the next release. (We've already merged some breaking changes into master.)
|
Great, thanks for the thoughts. Updated on comments, lmk any final changes! |
There was a problem hiding this comment.
I made a couple of small fixes. This looks ready to merge once CI passes.
|
Thanks @jturner314 ! |
|
Merge conflicts resolved |
ref #642 (comment)
Is this the right direction?
I'm only now understanding how the rust import system works; I initially didn't realize you'd need an additional
usestatement each time to usefrom_iter. Should we be using.collect()in the examples? Or adding thestd::iter::FromIterator;in thendarraystandard prelude?