Re: [Seaside-dev] GROrderedCollectionTest>>#testSort

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: [Seaside-dev] GROrderedCollectionTest>>#testSort

Lukas Renggli
> #sort you mean? Not yet... we either need to add it in Pharo, fix it
> in the one click, or add it to the Grease package.

OrderedCollection>>#sort and OrderedCollection>>#sort: is already part
of Pharo 1.1. The Seaside tests pass in Pharo 1.1.

I understand and I agree with Pharo not to implement #sort and #sort:
in SequenceableCollection, because it does not make sense for most
subclasses. SortedCollection, Heap and Interval are already sorted;
and for LinkedList sorting does not really make sense.

So I don't really know what to do to fix the Seaside test. We could
ask the Pharo guys to integrate OrderedCollection>>#sort and
OrderedCollection>>#sort: into Pharo 1.0, but maybe that's already too
late?

Lukas

--
Lukas Renggli
http://www.lukas-renggli.ch

_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Reply | Threaded
Open this post in threaded view
|

Re: [Seaside-dev] GROrderedCollectionTest>>#testSort

Marcus Denker-4

On Feb 17, 2010, at 2:13 PM, Lukas Renggli wrote:

>> #sort you mean? Not yet... we either need to add it in Pharo, fix it
>> in the one click, or add it to the Grease package.
>
> OrderedCollection>>#sort and OrderedCollection>>#sort: is already part
> of Pharo 1.1. The Seaside tests pass in Pharo 1.1.
>
> I understand and I agree with Pharo not to implement #sort and #sort:
> in SequenceableCollection, because it does not make sense for most
> subclasses. SortedCollection, Heap and Interval are already sorted;
> and for LinkedList sorting does not really make sense.
>
> So I don't really know what to do to fix the Seaside test. We could
> ask the Pharo guys to integrate OrderedCollection>>#sort and
> OrderedCollection>>#sort: into Pharo 1.0, but maybe that's already too
> late?
>

I think we can add that 1.0

For 1.1, there are some changes for generalizing and cleaning sort on the bugtracker that I plan to look
at (at some point).

        Marcus

--
Marcus Denker  -- http://www.marcusdenker.de
INRIA Lille -- Nord Europe. Team RMoD.


_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Reply | Threaded
Open this post in threaded view
|

Re: [Seaside-dev] GROrderedCollectionTest>>#testSort

Lukas Renggli
That would be great and would avoid that we have to conditionally load
methods depending on what version of Pharo is used. As far as I see
this is really just OrderedCollection>>#sort and
OrderedCollection>>#sort: and they have no external dependencies.

Lukas

On 17 February 2010 15:04, Marcus Denker <[hidden email]> wrote:

>
> On Feb 17, 2010, at 2:13 PM, Lukas Renggli wrote:
>
>>> #sort you mean? Not yet... we either need to add it in Pharo, fix it
>>> in the one click, or add it to the Grease package.
>>
>> OrderedCollection>>#sort and OrderedCollection>>#sort: is already part
>> of Pharo 1.1. The Seaside tests pass in Pharo 1.1.
>>
>> I understand and I agree with Pharo not to implement #sort and #sort:
>> in SequenceableCollection, because it does not make sense for most
>> subclasses. SortedCollection, Heap and Interval are already sorted;
>> and for LinkedList sorting does not really make sense.
>>
>> So I don't really know what to do to fix the Seaside test. We could
>> ask the Pharo guys to integrate OrderedCollection>>#sort and
>> OrderedCollection>>#sort: into Pharo 1.0, but maybe that's already too
>> late?
>>
>
> I think we can add that 1.0
>
> For 1.1, there are some changes for generalizing and cleaning sort on the bugtracker that I plan to look
> at (at some point).
>
>        Marcus
>
> --
> Marcus Denker  -- http://www.marcusdenker.de
> INRIA Lille -- Nord Europe. Team RMoD.
>
>
> _______________________________________________
> Pharo-project mailing list
> [hidden email]
> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>



--
Lukas Renggli
http://www.lukas-renggli.ch

_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Reply | Threaded
Open this post in threaded view
|

Re: [Seaside-dev] GROrderedCollectionTest>>#testSort

Marcus Denker-4

On Feb 17, 2010, at 3:17 PM, Lukas Renggli wrote:

> That would be great and would avoid that we have to conditionally load
> methods depending on what version of Pharo is used. As far as I see
> this is really just OrderedCollection>>#sort and
> OrderedCollection>>#sort: and they have no external dependencies.

Ok, I will do an update tonight.

        Marcus


--
Marcus Denker  -- http://www.marcusdenker.de
INRIA Lille -- Nord Europe. Team RMoD.


_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Reply | Threaded
Open this post in threaded view
|

Re: [Seaside-dev] GROrderedCollectionTest>>#testSort

Paolo Bonzini-2
In reply to this post by Lukas Renggli

> I understand and I agree with Pharo not to implement #sort and #sort:
> in SequenceableCollection, because it does not make sense for most
> subclasses. SortedCollection, Heap and Interval are already sorted;

Interval and SortedCollection are only sorted according to one
particular sort block, which may not be the default sort block (e.g. for
decreasing intervals).  For Interval the only sensible solution is
#shouldNotImplement, but for SortedCollection it is plausible to make
#sort or #sort: change the collection's sort block.

> and for LinkedList sorting does not really make sense.

Why not?  Unlike arrays you can do linked list mergesort without
auxiliary space (though the linked list already has O(n) overhead so
that means without extra space compared to what the data structure
already needs), and it is still guaranteed O(n log n).  Of course I'm
not really suggesting to implement LinkedList>>#sort. :-)

> So I don't really know what to do to fix the Seaside test. We could
> ask the Pharo guys to integrate OrderedCollection>>#sort and
> OrderedCollection>>#sort: into Pharo 1.0, but maybe that's already too
> late?

Grease-Squeak should provide it.

Paolo

_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Reply | Threaded
Open this post in threaded view
|

Re: [Seaside-dev] GROrderedCollectionTest>>#testSort

Julian Fitzell-2
In reply to this post by Lukas Renggli
On Wed, Feb 17, 2010 at 5:13 AM, Lukas Renggli <[hidden email]> wrote:

>> #sort you mean? Not yet... we either need to add it in Pharo, fix it
>> in the one click, or add it to the Grease package.
>
> OrderedCollection>>#sort and OrderedCollection>>#sort: is already part
> of Pharo 1.1. The Seaside tests pass in Pharo 1.1.
>
> I understand and I agree with Pharo not to implement #sort and #sort:
> in SequenceableCollection, because it does not make sense for most
> subclasses. SortedCollection, Heap and Interval are already sorted;
> and for LinkedList sorting does not really make sense.

As Paolo points out, they are not necessarily sorted by the desired
block. For SortedCollection, I agree the correct behaviour is to set
the sortBlock. VW handles subclasses (like Interval) that cannot be
sorted in place very simply: the sorter is implemented in terms of
#at: and #at:put: and classes that cannot be modified with #at:put:
just return an error saying that they can't be stored into. This also
covers, for example, Arrays that are marked as immutable. There are
many many subclasses of SequenceableCollection that do implement
#at:put: and can be sorted properly...

Julian

_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Reply | Threaded
Open this post in threaded view
|

Re: [Seaside-dev] GROrderedCollectionTest>>#testSort

Marcus Denker-4

On Feb 17, 2010, at 9:15 PM, Julian Fitzell wrote:

> On Wed, Feb 17, 2010 at 5:13 AM, Lukas Renggli <[hidden email]> wrote:
>>> #sort you mean? Not yet... we either need to add it in Pharo, fix it
>>> in the one click, or add it to the Grease package.
>>
>> OrderedCollection>>#sort and OrderedCollection>>#sort: is already part
>> of Pharo 1.1. The Seaside tests pass in Pharo 1.1.
>>
>> I understand and I agree with Pharo not to implement #sort and #sort:
>> in SequenceableCollection, because it does not make sense for most
>> subclasses. SortedCollection, Heap and Interval are already sorted;
>> and for LinkedList sorting does not really make sense.
>
> As Paolo points out, they are not necessarily sorted by the desired
> block. For SortedCollection, I agree the correct behaviour is to set
> the sortBlock.

This is done already in 1.1

> VW handles subclasses (like Interval) that cannot be
> sorted in place very simply: the sorter is implemented in terms of
> #at: and #at:put: and classes that cannot be modified with #at:put:
> just return an error saying that they can't be stored into. This also
> covers, for example, Arrays that are marked as immutable. There are
> many many subclasses of SequenceableCollection that do implement
> #at:put: and can be sorted properly...


Yes, and Niko therefore suggested to factor out the sorting method in a Trait  and
then import the Trait in all classes that have at: and at:put: (even LinkedList and Stack)

http://code.google.com/p/pharo/issues/detail?id=1346

        Marcus


--
Marcus Denker  -- http://www.marcusdenker.de
INRIA Lille -- Nord Europe. Team RMoD.


_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Reply | Threaded
Open this post in threaded view
|

Re: [Seaside-dev] GROrderedCollectionTest>>#testSort

Lukas Renggli
In reply to this post by Julian Fitzell-2
> covers, for example, Arrays that are marked as immutable. There are
> many many subclasses of SequenceableCollection that do implement
> #at:put: and can be sorted properly...

Yes, OrderedCollection and ArrayedCollection (already covered). For
all other subclasses the sort method does not make sense.

Lukas


--
Lukas Renggli
http://www.lukas-renggli.ch

_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Reply | Threaded
Open this post in threaded view
|

Re: [Seaside-dev] GROrderedCollectionTest>>#testSort

Julian Fitzell-2
On Wed, Feb 17, 2010 at 12:32 PM, Lukas Renggli <[hidden email]> wrote:
>> covers, for example, Arrays that are marked as immutable. There are
>> many many subclasses of SequenceableCollection that do implement
>> #at:put: and can be sorted properly...
>
> Yes, OrderedCollection and ArrayedCollection (already covered). For
> all other subclasses the sort method does not make sense.

Ok, that's fine. The Grease tests are only testing ANSI collections
anyway and, for #sort:, currently only classes that implement
<sequencedCollection> (Interval does not). We should added
SortedCollection to that test (but I see there's no TestCase for
SortedCollection yet).

I thought you were saying we were testing all subclasses of
SequencedCollection, but that is not the case (and should not be).

Julian

_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project