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 findmax(A,dim) function to array.jl. #6816

Closed
wants to merge 1 commit into from
Closed

added findmax(A,dim) function to array.jl. #6816

wants to merge 1 commit into from

Conversation

floswald
Copy link

hi all,

this is my attempt at implementing my original feature request for a version of findmax that operates along a given index of an array.

I added some tests to test/reducedim.jl, although not entirely sure that's where they belong and how this needs to be set up to work with the full test suite.

@timholy
Copy link
Sponsor Member

timholy commented May 12, 2014

Thanks for tackling this! I'll add a few comments.

@floswald
Copy link
Author

Thanks for the comments Tim! So much to learn!

On 12 May 2014 13:32, Tim Holy notifications@github.com wrote:

Thanks for tackling this! I'll add a few comments.


Reply to this email directly or view it on GitHubhttps://github.com//pull/6816#issuecomment-42825964
.

@timholy
Copy link
Sponsor Member

timholy commented May 12, 2014

It's one of the best things about submitting patches. I've learned a ton from my colleagues here.

@timholy
Copy link
Sponsor Member

timholy commented May 18, 2014

@floswald, I don't mean to stand in the way of progress. I've got a free hour or two now, if you prefer I can pitch in. But if you're having fun learning, I certainly wouldn't want to take that away from you 😄. Keep the questions coming if you've got them.

@floswald
Copy link
Author

Oh no please by all means do pitch in! I'm quite eager to see the proper solution to this problem. :-)
I'll come back with questions later if that's alright. Cheers!

Sent from my iPhone

On 18 May 2014, at 15:11, Tim Holy notifications@github.com wrote:

@floswald, I don't mean to stand in the way of progress. I've got a free hour or two now, if you prefer I can pitch in. But if you're having fun learning, I certainly wouldn't want to take that away from you . Keep the questions coming if you've got them.


Reply to this email directly or view it on GitHub.

@timholy
Copy link
Sponsor Member

timholy commented May 20, 2014

See #6894

@floswald
Copy link
Author

brilliant! thank you so much!
do you know when this piece of code will be available on the julia master branch (or is that not the right place to look for it)? cheers

@floswald floswald closed this May 21, 2014
@ViralBShah
Copy link
Member

Yes, it will be on master as soon as the PR is merged.

@timholy
Copy link
Sponsor Member

timholy commented May 21, 2014

Now available. Enjoy.

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