On Jun 23, 2012, at 4:03 AM, Sergiu Ivanov <[email protected]> wrote:
> On Sat, Jun 23, 2012 at 10:53 AM, Aaron Meurer <[email protected]> wrote: >> On Jun 23, 2012, at 1:41 AM, Sergiu Ivanov <[email protected]> >> wrote: >> >>> On Sat, Jun 23, 2012 at 2:07 AM, Aaron Meurer <[email protected]> wrote: >>>> Without getting into the discussion about Basic args, it should be >>>> possible to sort the args in a platform independent way. For example, >>>> with str or __name__ (str should work unless it's a class that hasn't >>>> overridden it, which will have the id in the string). >>> >>> Yes, indeed; however, that would make the impact of creating a >>> FiniteSet even more serious, because doing str often requires >>> traversing the expression tree. >> >> I mean just using it for non-Basic types. > > Oh, I see, that sounds great. > >>>> My concern right now is more about getting tests to pass with hash >>>> randomization than with architectural issues, so that we can release. >>>> So I'd gladly see some quick fixes to get the tests to pass now, and >>>> we can work out a better architecture later. >>> >>> I'll to try to drop the sorting right now and see how I can fix the >>> things that get broken (if something indeed gets so). >> >> Of course, if you can do this, it will be even better. > > I'm coming back with not-that-good news. > > FiniteSet creates a frozenset internally, to drop duplicates. If we > just convert that to a list, we get essentially arbitrary order of > elements in the FiniteSet. This is going to randomly break every > single doctest showing FiniteSet. I guess this was the initial > thought behind sorting the elements of the FiniteSet. > > There are two immediately available ways out: > > 1. Don't use frozenset to remove duplicates; instead, remove them by > hand, preserving the order of the elements as they were supplied to > the constructor. This will result in O(1/2 n^2) complexity of > creating FiniteSet, which is most probably worse than what it is at > the moment. > > 2. Have FiniteSet sort its elements, just as it does now. However, > fix that one doctest which puts *two* non-Basic objects in a > FiniteSet to only put *one* non-Basic object. This will convey the > same message and avoid the dependency on the machine. Note that > the failures in the doctests now are only due to the fact that the > two non-Basic elements in one single doctests are getting sorted in > different order with respect to each other, while all other > elements always get arranged in the same way. > > I'd vote for the second approach, because it's fastest and simplest. > Note that, apparently, there is only one doctest in the whole SymPy > which places two non-Basic objects in a FiniteSet and thus causes > trouble. Also note that the problem we are currently facing only > appears in the doctests. I would change it to hold zero non-Basic objects. That way at least the behavior won't be documented and we can remove it without much issue in the future. And by the way, .args doesn't have to be sorted for the printing order to be sorted. Aaron Meurer > > This however does not solve the important questions raised in this > thread. I'm still not sure whether it's good that FiniteSet is able > to store anything. It's still not a good idea to actually sort things > in FiniteSet. I guess I could open issues for the two, if no one > objects. > > Sergiu > > -- > You received this message because you are subscribed to the Google Groups > "sympy" group. > To post to this group, send email to [email protected]. > To unsubscribe from this group, send email to > [email protected]. > For more options, visit this group at > http://groups.google.com/group/sympy?hl=en. > -- You received this message because you are subscribed to the Google Groups "sympy" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/sympy?hl=en.
