-
Notifications
You must be signed in to change notification settings - Fork 22
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
Switch from trove to fastutil #54
base: master
Are you sure you want to change the base?
Conversation
@@ -114,7 +114,8 @@ | |||
* @return the array of distinct values in the order they were observed | |||
*/ | |||
public static Array<Integer> distinct(IntStream values, int limit) { | |||
final DistinctInts distinct = new DistinctInts(limit); | |||
final int capacity = limit < it.unimi.dsi.fastutil.Arrays.MAX_ARRAY_SIZE ? limit : 1000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we throw a new IllegalArgumentException here if limit > MAX_ARRAY_SIZE? I'm wondering if it'll be confusing users if we use a value that's different than what they passed in
@Zavster I'd love to see this PR make its way in after all the work that's been done on it already! :-) |
@Zavster any reason not to merge this? |
Hey Ben, I wanted to assess using Goldman Sachs collection as a possible alternative, but have been working on other parts of Morpheus lately. I will definitely adopt either GS collections or Fastutil, hopefully before year end. |
Any thoughts on this one? |
@Zavster think you might still merge this? |
Hey Ben, I am sitting out a non-compete which ends in early September. I will resume Morpheus development in anger then.... |
Good luck with the new gig! I've been contributing quite a bit to Tablesaw in the meantime. I'm not quite sure what it'd look like, but it'd be cool to join forces on the libraries somehow |
No description provided.