roarin-roran / Sorting

0 stars 0 forks source link

test for stability #46

Open roarin-roran opened 2 years ago

roarin-roran commented 2 years ago

currently, the search is only tested for correctness with integer inputs - more infrastructure would be needed to ensure that our methods are stable in practice, as well as an addition to the test_sorters unit test file.

@sebawild I'd appreciate a hand prioritizing this issue - is it something we definitely need to do eventually? is it something that needs to be done soon?

sebawild commented 2 years ago

It is a good thing to have, and probably not hard to do with a custom type

sebawild commented 2 years ago

But not urgent either; stability is easy to check "by inspection"

roarin-roran commented 2 years ago

yeah, we'd just need to switch from sorting ints to sorting lightweight objects that remember their original order. not hard to do, but I would probably have to read the entire code base looking for places where the value of an input element and the contents of that element are assumed to be the same

roarin-roran commented 2 years ago

will hold off on the small fry label, but not plan on doing this yet.

roarin-roran commented 2 years ago

update methods to sort objects with a key, to force comparable memory access between cpy and rpy

sebawild commented 2 years ago

How hard would it be to follow the design of the library method, i.e., optionally accept a key extractor function? The other option should be to override the comparison operator. Should have code for that, let me check.

sebawild commented 2 years ago

Here we go

class Wrapper:

    def __init__(self, key):
        self._key = key

    def __lt__(self, other):
        return self._key < other._key

wrapped_list = [Wrapper(i) for i in some_list]
sebawild commented 2 years ago

This would only allow <, but one can let Python auto-generate the other comparison operations with this: https://docs.python.org/3/library/functools.html#functools.total_ordering

@total_ordering
class Wrapper:
# rest as before

    def __init__(self, key):
        self._key = key

    def __lt__(self, other):
        return self._key < other._key
roarin-roran commented 2 years ago

That seems like a good way to approach it