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

Comparison operator in findmin() variants #14216

Open
juliohm opened this issue Dec 1, 2015 · 18 comments
Open

Comparison operator in findmin() variants #14216

juliohm opened this issue Dec 1, 2015 · 18 comments
Labels
collections Data structures holding multiple items, e.g. sets help wanted Indicates that a maintainer wants help on an issue or pull request

Comments

@juliohm
Copy link
Contributor

juliohm commented Dec 1, 2015

This issue was raised in the mailing list: https://groups.google.com/forum/#!topic/julia-users/iBydnBqQgxg

Although we can define a custom type and specialize the < operator for it, sometimes we may be interested in testing different operators on the spot. What are your thoughts on an additional option a la C++?

findmin(array, lt=<)
@stevengj
Copy link
Member

stevengj commented Dec 1, 2015

It should probably be like sort and also take a by? And shouldn't the same options apply to the minimum function?

@juliohm
Copy link
Contributor Author

juliohm commented Dec 3, 2015

@stevengj I'm planning to add just this option for now, others can contribute more options in the future if they find it useful. Can I move forward and send a PR?

@juliohm
Copy link
Contributor Author

juliohm commented Dec 7, 2015

Actually I'm not sure if that is a good addition. From one side it looks like an useful option, from the other side I think one can always convert the set of objects to a numeric score with a simple map, apply indmin and query the desired object.

What is more Julian and clean?

@juliohm
Copy link
Contributor Author

juliohm commented Dec 7, 2015

Yes, I am closing this one as non-Julian.

@juliohm juliohm closed this as completed Dec 7, 2015
@stevengj
Copy link
Member

stevengj commented Dec 7, 2015

Actually, I think by and lt keywords, analogous to sort, would be a good idea in findmin, minimum, and median. Avoiding allocation of additional arrays, not to mention consistency with sort, is typically Julian.

@eschnett
Copy link
Contributor

eschnett commented Dec 7, 2015

You are assuming that each collection is indexable. This doesn't work e.g. with dictionaries:

julia> d = Dict(3=>'a', 2=>'b')
Dict{Int64,Char} with 2 entries:
  2 => 'b'
  3 => 'a'

julia> findmin(d)
(2=>'b',1)

julia> d[1]
ERROR: KeyError: 1 not found

That is, one first has to convert the dictionary to an array while converting the element types, then apply findmin, then get the respective array element. It is much more efficient to write a for loop while traversing the dictionary.

@eschnett eschnett reopened this Dec 7, 2015
@juliohm
Copy link
Contributor Author

juliohm commented Dec 7, 2015 via email

juliohm added a commit to juliohm/julia that referenced this issue Dec 7, 2015
@stevengj
Copy link
Member

stevengj commented Dec 8, 2015

@juliohm, please look at how by and lt are implemented for sort and use the same mechanism.

@juliohm
Copy link
Contributor Author

juliohm commented Jul 2, 2017

@stevengj could you please add the label up for grabs and intro issue to this issue?

@Sacha0 Sacha0 added help wanted Indicates that a maintainer wants help on an issue or pull request collections Data structures holding multiple items, e.g. sets labels Jul 2, 2017
@Sacha0
Copy link
Member

Sacha0 commented Jul 2, 2017

Lovely meeting you last week Julio! :) I added the up for grabs label, but forewent adding the intro issue label (which I am not certain is accurate for this project). Best!

@juliohm
Copy link
Contributor Author

juliohm commented Jul 2, 2017

The same Sacha! Thanks for adding the label, we should set a meetup at Stanford for Julia users at some point. :) Best!

@juliohm
Copy link
Contributor Author

juliohm commented Jan 3, 2018

I believe this issue is being addressed somewhere else already for Julia v0.7? If it is a duplicate, could you please close it?

@nalimilan
Copy link
Member

I'm not aware of changes which would have fixed this at this point.

@juliohm
Copy link
Contributor Author

juliohm commented Jan 3, 2018

Thank you @nalimilan, plans to address this issue in your recent PR (#24673) or in the 1.0 milestone?

@nalimilan
Copy link
Member

No, the priority for 0.7 is to make the API consistent, new features can be added later.

@juliohm
Copy link
Contributor Author

juliohm commented Jan 3, 2018

Makes sense, this issue can be viewed as a new feature since the option is not implemented yet.

@juliohm
Copy link
Contributor Author

juliohm commented Mar 23, 2019

Maybe we could add this issue to some milestone? Like Julia 1.2?

@nalimilan
Copy link
Member

If somebody does the work it can be merged at any point, but we use time-based releases rather than feature-based releases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
collections Data structures holding multiple items, e.g. sets help wanted Indicates that a maintainer wants help on an issue or pull request
Projects
None yet
Development

No branches or pull requests

5 participants