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

treat Pair as broadcast scalar #32209

Merged
merged 1 commit into from
Aug 1, 2019
Merged

treat Pair as broadcast scalar #32209

merged 1 commit into from
Aug 1, 2019

Conversation

stevengj
Copy link
Member

As discussed on discourse, it seems much more useful to treat Pair as a scalar for broadcast. (There are other iterable objects, e.g. String, that are also broadcast scalars.)

The example on the mailing list was replace.(["string1", "string2", ...], pat=>rep), and one can come up with many others. Conversely, I find it hard to imagine any case where iterating over the pair would be useful in a broadcast context, especially since the output is an array rather than another Pair:

julia> .-(3 => 4)
2-element Array{Int64,1}:
 -3
 -4

Hence I'm marking this as a "minor change" — technically breaking, but unlikely to affect real code.

@stevengj stevengj added broadcast Applying a function over a collection minor change Marginal behavior change acceptable for a minor release labels May 31, 2019
@mbauman
Copy link
Sponsor Member

mbauman commented May 31, 2019

Yeah, this is probably the way to go here. It's unlikely that it's useful to broadcast the same function over both sides of the pair. It is definitely breaking, though.

@stevengj
Copy link
Member Author

CI failures are unrelated #32186.

@mbauman mbauman added the triage This should be discussed on a triage call label May 31, 2019
@StefanKarpinski
Copy link
Sponsor Member

Comment from triage (Jeff): it's probably not great that pairs are even iterable and we probably only allow it for destructuring. Triage is in favor of changing this.

@StefanKarpinski StefanKarpinski added this to the 1.3 milestone Jun 6, 2019
@StefanKarpinski StefanKarpinski removed the triage This should be discussed on a triage call label Jun 6, 2019
@stevengj
Copy link
Member Author

stevengj commented Jun 11, 2019

Rebased to fix NEWS conflict. Should be good to merge.

@stevengj
Copy link
Member Author

stevengj commented Aug 1, 2019

I'm going to go ahead and merge shortly since triage was in favor and no one has objected to this; it seems to have fallen under the radar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
broadcast Applying a function over a collection minor change Marginal behavior change acceptable for a minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants