Skip to content

Conversation

@JeffBezanson
Copy link
Member

This is a re-do of #12128. This time includes Ranges and some tests.

Another example:

Before:

julia> ['a':'z', 1:2]
2-element Array{StepRange{Any,Int64},1}:
 'a':1:'z'
 1:1:2    

after:

julia> ['a':'z', 1:2]
2-element Array{StepRange{T,Int64} where T,1}:
 'a':1:'z'
 1:1:2    

julia> map(typeof, ans)
2-element Array{DataType,1}:
 StepRange{Char,Int64} 
 StepRange{Int64,Int64}

@JeffBezanson JeffBezanson added arrays [a, r, r, a, y, s] breaking This change will break code collections Data structures holding multiple items, e.g. sets labels Jul 11, 2017
StepRange{promote_type(T1a,T2a),promote_type(T1b,T2b)}
promote_rule(::Type{StepRange{T1a,T1b}}, ::Type{StepRange{T2a,T2b}}) where {T1a,T1b,T2a,T2b} =
el_same(promote_type(T1a,T2a),
StepRange{T1a, promote_type(T1b,T2b)},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a or b here? could use comments

@JeffBezanson
Copy link
Member Author

@nanosoldier runbenchmarks(ALL, vs = ":master")

@nanosoldier
Copy link
Collaborator

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

@ararslan
Copy link
Member

I'm inclined to think that the NullableArray regression is real, given how much promotion stuff those require.

@JeffBezanson
Copy link
Member Author

I can't reproduce it locally, and there doesn't seem to be much array promotion happening there. I think it's fine.

@ararslan
Copy link
Member

ararslan commented Jul 24, 2017

¯\_(ツ)_/¯ Okay

@ararslan ararslan merged commit 04d76f3 into master Jul 24, 2017
@ararslan ararslan deleted the jb/elpromote branch July 24, 2017 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arrays [a, r, r, a, y, s] breaking This change will break code collections Data structures holding multiple items, e.g. sets

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants