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

Add endof for Pair #12335

Merged
merged 1 commit into from
Jul 28, 2015
Merged

Add endof for Pair #12335

merged 1 commit into from
Jul 28, 2015

Conversation

joehuchette
Copy link
Member

first(::Pair) already works on master, this adds the corresponding last method.

@mlubin
Copy link
Member

mlubin commented Jul 28, 2015

Ref #12261 which broke JuMP

@@ -441,6 +441,9 @@ getindex(p::Pair,i::Int) = getfield(p,i)
getindex(p::Pair,i::Real) = getfield(p, convert(Int, i))
reverse(p::Pair) = Pair(p.second, p.first)

first(p::Pair) = p.first
Copy link
Member

Choose a reason for hiding this comment

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

Why add this definition if first(::Pair) already works?

Copy link
Member Author

Choose a reason for hiding this comment

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

Only for clarity

@joehuchette
Copy link
Member Author

Alternatively, I think this could be implemented by adding endof(::Pair).

@joehuchette joehuchette changed the title Add first/last for Pair Add endof for Pair Jul 28, 2015
@mlubin
Copy link
Member

mlubin commented Jul 28, 2015

@joehuchette, could you squash?

@mlubin
Copy link
Member

mlubin commented Jul 28, 2015

Are there any objections to this? Since you can already do (:a => :b)[1] and (:a => :b)[2], we're just adding (:a => :b)[end].

@tkelman
Copy link
Contributor

tkelman commented Jul 28, 2015

this lgtm, ci's a bit backed up since there were a lot of merges today

@jakebolewski
Copy link
Member

Should help

Liver Salt

mlubin added a commit that referenced this pull request Jul 28, 2015
@mlubin mlubin merged commit 53aabc0 into master Jul 28, 2015
@tkelman tkelman deleted the jh/last-pair branch July 28, 2015 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants