Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: Collect to Vector #47777

Open
jakobnissen opened this issue Dec 2, 2022 · 6 comments
Open

Feature request: Collect to Vector #47777

jakobnissen opened this issue Dec 2, 2022 · 6 comments
Labels
domain:arrays [a, r, r, a, y, s]

Comments

@jakobnissen
Copy link
Contributor

As far as I can tell, there is currently no idiomatic way of collecting an arbitrary iterable into a Vector{T}. To me this seems like a basic operation. At least, I need it quite often.

  • collect does not always return a Vector
  • Array comprehensions calls collect on a Generator, which preserves shape information in some cases. Hence, it does not always return Vector
  • vec(collect(itr)) returns an array with shared memory. I believe this can be problematic sometimes, but I'm not completely sure.
  • [i for i in itr if true] does work, but is not exactly idiomatic. Also, I don't think it's documented to return a Vector.
@StefanKarpinski
Copy link
Sponsor Member

This is what collect was meant to do but people decided to make it do other random stuff based on its argument type.

@quinnj
Copy link
Member

quinnj commented Dec 2, 2022

Related: #36537

@brenhinkeller brenhinkeller added the domain:arrays [a, r, r, a, y, s] label Dec 2, 2022
@bvdmitri
Copy link
Contributor

bvdmitri commented Dec 3, 2022

What about convert(Vector, ...)?

@jakobnissen
Copy link
Contributor Author

That's not how convert is meant to be used. Maybe the vector constructor.

@bvdmitri
Copy link
Contributor

bvdmitri commented Dec 3, 2022

That's not how convert is meant to be used.

Can you elaborate on that? The official documentation gives no hints on why this usage of convert might be wrong? convert(T, ...) should always return a value of type T, while T(...) does not necessarily returns a value of type T.

@jakobnissen
Copy link
Contributor Author

jakobnissen commented Dec 3, 2022

@bvdmitri See the docs on conversion vs construction. Convert is only meant to be called when converting between the "same kind of things", e.g. an UInt8 and a UInt64 both encode a number (the same kind of thing). It's also supposed to be lossless. Similar with Vector{Int} and UnitRange{Int}. However, collect should be able to take arbitrary iterables, which are not necessarily equivalent to a Vector

Futhermore, since convert is called implicitly, I would favor implementing methods for convert only when absolutely uncontroversial, and always err on the side of not implementing convert. I've been bitten a few times by unintended calls to convert.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:arrays [a, r, r, a, y, s]
Projects
None yet
Development

No branches or pull requests

5 participants