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

added extrema(A,dims) #15550

Merged
merged 1 commit into from
Mar 22, 2016
Merged

added extrema(A,dims) #15550

merged 1 commit into from
Mar 22, 2016

Conversation

bjarthur
Copy link
Contributor

minimum and maximum already have a dims option, but not extrema, so i added it. see #15376 and #3893 for discussion.

@@ -2083,11 +2083,18 @@ appended to an internal buffer of backtraces.
:@profile

"""
extrema(itr)
extrema(itr) -> Tuple
Copy link
Contributor

Choose a reason for hiding this comment

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

you need to adjust the rst signature to match this change, add the new signature to the rst, run make docs and commit the generated rst

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i'm sorry, but could you be more specific? how do i "add the new signature to the rst"? thanks.

@bjarthur
Copy link
Contributor Author

@tkelman docs are now fixed

@bjarthur bjarthur force-pushed the extrema branch 3 times, most recently from 97b93d2 to a44abb4 Compare March 19, 2016 19:12
@tkelman
Copy link
Contributor

tkelman commented Mar 21, 2016

Looks okay to me but could use a correctness double check from someone familiar with the cartesian macros.

(@nref $N B j) = (BJ[1], AI)
end
end
squeeze(B, tuple(find([sB...].==1)...))
Copy link
Member

Choose a reason for hiding this comment

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

I'd get rid of this squeeze for consistency with minimum(A, dims), maximum(A, dims), etc.

@bjarthur
Copy link
Contributor Author

squeeze removed

@timholy
Copy link
Member

timholy commented Mar 22, 2016

Looks good, thanks!

timholy added a commit that referenced this pull request Mar 22, 2016
@timholy timholy merged commit 59858c6 into JuliaLang:master Mar 22, 2016
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.

3 participants