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

Make Broadcasted iterable and more indexable #26987

Merged
merged 1 commit into from
May 9, 2018
Merged

Conversation

mbauman
Copy link
Sponsor Member

@mbauman mbauman commented May 4, 2018

Defines iteration and some related traits as well as a few functions to make indexing a bit friendlier.

Defines iteration and some related traits as well as a few functions to make indexing a bit friendlier.
@mbauman mbauman added the domain:broadcast Applying a function over a collection label May 4, 2018
@mbauman
Copy link
Sponsor Member Author

mbauman commented May 7, 2018

Any comments here? I feel like this should be largely unobjectionable, but I do want to point to my comment here #19198 (comment): defining iteration pretty much locks us into the status quo on the evaluation strategy.

Travis 32bit failure was a bunch of ProcessExitedException()s — maybe OOMs? Appveyor looks like timeouts.

@andyferris
Copy link
Member

FWIW I agree more-or-less completely with #19198 (comment), especially

While it feels like we should be able to refactor Broadcasted to work purely based upon iteration alone (and not indexing), I think it'd be really hard and possibly impossible for some combinations of iterable arguments. Doing so performantly is even worse.

@StefanKarpinski
Copy link
Sponsor Member

I think the breakdown of map is for iterable things and broadcast is for indexable things furthers that position as well, so 👍. Another potential use for this is lazy construction of a broadcasted object which you could then do random access indexing into if you don't actually want to materialize the entire broadcasted array, which seems like it could be very handy sometimes, as well as intuitive. That seems at odds with an iteration-based conception of broadcasted objects.

@mbauman
Copy link
Sponsor Member Author

mbauman commented May 9, 2018

Great. Let's re-run CI now that I think things are back in working order.

@mbauman mbauman closed this May 9, 2018
@mbauman mbauman reopened this May 9, 2018
@mbauman
Copy link
Sponsor Member Author

mbauman commented May 9, 2018

Travis Mac failed with a GitError(Code:ERROR, Class:Net, curl error: Could not resolve host: github.com.

@mbauman mbauman merged commit 0db674d into master May 9, 2018
@mbauman mbauman deleted the mb/betterbroadcasteds branch May 9, 2018 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:broadcast Applying a function over a collection
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants