new list and set functions #373
Conversation
…[] and DeleteDuplicates[]
| return | ||
|
|
||
| if list.get_head_name() != 'System`List': | ||
| evaluation.error(self.get_name(), 'needlist') |
There was a problem hiding this comment.
should be return evaluation.message(self.get_name(), 'list', expr, 1)
There was a problem hiding this comment.
I haven't understood the difference between evaluation.message and evaluation.error yet. Is message more like a warning or is it also used for errors? I'm just wondering if the thing above should be an evaluation.error after all?
There was a problem hiding this comment.
evaluation.message acts as both a warning and an error message. It's used whenever evaluation can continue. Normally continuing just means return the expression unevaluated. In this case for example, there is no evaluation 'error', it's just that the function doesn't apply to all inputs.
evaluation.error on the other hand is used when there is an error with the evaluation process itself. For example, if the recursion depth gets above $RecursionLimit then that's an evaluation error since evaluation can't continue. We only use evauation.error when the calculation has to be aborted.
We try to keep the return values and messages compatible with Mathematica:
In[1]:= Gather[a + b + c + a]
Gather::list: List expected at position 1 in Gather[2 a + b + c].
Out[1]= Gather[2 a + b + c]Does that make sense?
There was a problem hiding this comment.
Ok, I think I understand, so it usually be evaluation.message.
|
I redid the evaluation.message stuff, and I added additional checks in SortBy[] with an error message guarding against bad things happening in the Map[]. |
|
Okay, I'll have a final look over tomorrow but I think this looks good to merge. |
|
I think a better place for the hash tests would be e.g. |
|
Moved there now. And it runs without explicitly hooking it up. This is nice. |
|
Some performance verification for |
| yield ea | ||
|
|
||
|
|
||
| class Intersect(_SetOperation): |
There was a problem hiding this comment.
Minor typo: the name of this function is Intersection.
| Bin = self._bin | ||
|
|
||
| select = equivalence.select() | ||
| same = equivalence.same() |
There was a problem hiding this comment.
Unless I'm missing something, the usage of .select() and .same() here seems more complicated than it needs to be. I don't think these need to return closures here.
How about refactoring these methods so this function can just call equivalance.select(elem) and equivalence.same(prototype, elem)? This would also allow _make_test_pair() to be folded into _SlowEquivalence.same().
There was a problem hiding this comment.
I agree that the code is less readable than it should be here. I'm always concerned about Python name lookups in inner loops, that's why I came up with these lambdas in the first place, but I think the code's much cleaner without them. Changed accordingly.
|
I'll do some final tests with this but I think it's looking good to merge. |
|
One minor issue I found with |
Union/Intersection arg duplicates custom sametest fix
This implements Gather[], GatherBy[], Tally[], Union[], Intersect[], IntersectingQ[], DisjointQ[].
It also updates the existing implementations of Complement[] and DeleteDuplicates[] so that they work in n log n if SameTest is SameQ (as opposed to the old implementations who where n^2).
It also implements SortBy[] and BinarySearch[].