Implementing merge sort function in python class, errors

Question

So I have a function defined that works great at doing merge sort on a linear array if it is implemented by its lonesome, but if I put it into a class it bugs out. I think it's a great example of what I don't quite understand about how classes work; possibly in regards to namespace management(?).

See below:

def sort(array):
    print('Splitting', array)
    if len(array) > 1:
        m = len(array)//2
        left = array[:m]
        right = array[m:]

        sort(left)
        sort(right)

        i = 0
        j = 0
        k = 0

        while i < len(left) and j < len(right):
            if left[i] < right[j]:
                array[k] = left[i]
                i += 1
            else:
                array[k] = right[j]
                j += 1
            k += 1

        while i < len(left):
            array[k] = left[i]
            i += 1
            k += 1

        while j < len(right):
            array[k] = right[j]
            j += 1
            k += 1
    print('Merging', array)

arr = [1,6,5,2,10,8,7,4,3,9]
sort(arr)

Produces the expected correct output:

Splitting  [1, 6, 5, 2, 10, 8, 7, 4, 3, 9]
Splitting  [1, 6, 5, 2, 10]
Splitting  [1, 6]
Splitting  [1]
Merging  [1]
Splitting  [6]
Merging  [6]
Merging  [1, 6]
Splitting  [5, 2, 10]
Splitting  [5]
Merging  [5]
Splitting  [2, 10]
Splitting  [2]
Merging  [2]
Splitting  [10]
Merging  [10]
Merging  [2, 10]
Merging  [2, 5, 10]
Merging  [1, 2, 5, 6, 10]
Splitting  [8, 7, 4, 3, 9]
Splitting  [8, 7]
Splitting  [8]
Merging  [8]
Splitting  [7]
Merging  [7]
Merging  [7, 8]
Splitting  [4, 3, 9]
Splitting  [4]
Merging  [4]
Splitting  [3, 9]
Splitting  [3]
Merging  [3]
Splitting  [9]
Merging  [9]
Merging  [3, 9]
Merging  [3, 4, 9]
Merging  [3, 4, 7, 8, 9]
Merging  [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]
[1, 2, 3, 4, 5, 6, 7, 8, 9, 10]

However, I get an error when I attempt to use this function in a class; something to do with namespace managment, I think. See below:

class MergeSort(object):

    def __init__(self, array):
        self.array = array

    def sort(self):
        print('Splitting', self.array)
        if len(self.array) > 1:
            m = len(self.array)//2
            left = self.array[:m]
            right = self.array[m:]

            sort(left)
            sort(right)

            i = 0
            j = 0
            k = 0

            while i < len(left) and j < len(right):
                if left[i] < right[j]:
                    self.array[k] = left[i]
                    i += 1
                else:
                    self.array[k] = right[j]
                    j += 1
                k += 1

            while i < len(left):
                self.array[k] = left[i]
                i += 1
                k += 1

            while j < len(right):
                self.array[k] = right[j]
                j += 1
                k += 1
        print('Merging', self.array)

x = MergeSort([1,6,5,2,10,8,7,4,3,9])
x.sort()

Produces the error output:

Splitting [1, 6, 5, 2, 10, 8, 7, 4, 3, 9]
---------------------------------------------------------------------------
NameError                                 Traceback (most recent call last)
<ipython-input-15-89509f86277e> in <module>()
      1 x = MergeSort([1,6,5,2,10,8,7,4,3,9])
----> 2 x.sort()

<ipython-input-14-2bba116f00ce> in sort(self)
     11             right = self.array[m:]
     12 
---> 13             sort(left)
     14             sort(right)
     15 

NameError: name 'sort' is not defined

My initial instinct, after google searching around was to change subroutines sort(left) and sort(right) by adding prefixive self., but that generates a positional argument error. Would love a comment or two on what it is that I'm not understanding here. And cheers for good votes if my question is not stupid, and down votes if it is.

Your recursive calls to sort should be `self.sort`. So, it should not be `sort(left)` and `sort(right)`, but `self.sort(left)` and `self.sort(right)`. — Tobias Brösamle, Jun 08 '18 at 06:31
@TobiasBrösamle No, that won't work. `sort` doesn't take an argument (besides `self`). And, as the OP said in the question, they already tried "adding prefixive self". — abarnert, Jun 08 '18 at 06:34
@TobiasBrösamle if I do that I get the positional argument error. — reka18, Jun 08 '18 at 06:34
By the way, describing what you wrote instead of showing the code, and then describing the error instead of showing the actual exception, tends to lead to people misunderstanding your question unless they read it very carefully (as seen in the first comment and one of the answers here), and also makes it impossible for people to debug it if you made a simple mistake that might be obvious to others but not to you. — abarnert, Jun 08 '18 at 06:41

score 4 · Accepted Answer · edited Jun 08 '18 at 07:01

4

The reason sort(left) doesn't work is that, as you surmised, you can't call a method on self without specifying self. Leaving that off means it looks for a local or global name sort, doesn't find one, and raises a NameError.

The reason self.sort(left) doesn't work is that the API you defined doesn't work that way. Your class takes the list as a constructor argument, and then takes a sort with no arguments, that operates on the list passed in at construction time. So, you have no way to call your own sort with a different array. If you try self.sort(left), you're passing the wrong number of arguments, just like calling abs(1, 2), and you get the same TypeError.

You have to use your API the way you designed it: Create a new MergeSort sorter object with the new list, then call sort on that new object:

leftsorter = MergeSort(left)
leftsorter.sort()
rightsorter = MergeSort(right)
rightsorter.sort()

edited Jun 08 '18 at 07:01

bruno desthuilliers

75,974
6
88
118

answered Jun 08 '18 at 06:34

abarnert

354,177
51
601
671

Essentially there is no functional reason to put this function into a class, but I have been reviewing my understanding of how classes work. As an exercise, sometimes in futility, I have been working functions I've created previously into classes to see how they behave. And voila I encountered something unexpected. Good to have this community. I think I'll gain something from this thread for sure. – reka18 Jun 08 '18 at 06:42
1

@data83 This is probably about as good a use for classes as you could come up with for this function. It's just that there really isn't any behavior in a merge sort other than the actual sorting, and there isn't any persistent state other than the list itself, so it's not the best example of something to turn into a class. If you can find a function that (maybe after refactoring into a bunch of small functions) has state that needs to persist between function calls, that might be a better candidate. But anyway, it doesn't hurt to try everything, even if some things don't pan out… – abarnert Jun 08 '18 at 06:47
I'm going to take a crack at Strassen's subcubic matrix multiplication algorithm in a class. – reka18 Jun 08 '18 at 07:01
@data83, while this looks like an excercise, please note than none of your designs return the sorted list, which is presumably the idea of having it sorted – Evgeny Jun 08 '18 at 07:23
1

@EvgenyPogrebnyak His designs modify the list to be sorted. That's a perfectly reasonable thing to do. And they should not return the sorted list if they do so. – abarnert Jun 08 '18 at 07:26
1

That is solved by adding `return(self.array)` to the last line of the sort( ) function within the class. – reka18 Jun 08 '18 at 07:28
1

@data83 First, it's not a problem, so it shouldn't be solved. And it's particularly misleading in a recursive function, where the recursive calls have nothing useful to do with the return value. – abarnert Jun 08 '18 at 07:48
@abarnert what I meant was, and how I implemented it in my answer, was that I return the final array outside the loop. Was Evgeny suggesting I return the array at each point during the recursion? If so, then yeah I don't see how that is useful. – reka18 Jun 08 '18 at 07:51
1

@data83 My point is, I don’t think you want to return the list here at all. The caller still has access to `x.array`. And the usual Python idiom is that if the main point of a function is to mutate your self or a parameter, you return `None`, as explained somewhere in the official FAQ, but I’m no longer in front of my computer. – abarnert Jun 08 '18 at 08:08
1

`x.array` boom shakalaka, yeah I see your point. It's redundant to have the function return as part of a class. Cheers – reka18 Jun 08 '18 at 08:26
1

@abarnert Please see https://stackoverflow.com/questions/50774975/python-implementing-recursive-functions-in-classes-isnt-intuitive and thank you for your help. You helped me understand how to implement recursion in python classes and I'm very greatful. Cheers! – reka18 Jun 09 '18 at 13:56

reka18 · Answer 2 · 2018-06-08T08:27:33.950

Replacing the sort(left) and sort(right) components of the sort() function within my class with

leftsorter = MergeSort(left)
leftsorter.sort()
rightsorter = MergeSort(right)
rightsorter.sort()

(Thank you abarnert)

While at the same time removing debugging print statements from the code (thank you Evgany P), and by avoiding reusing a built in function name sort() to avoid confusion, I have a working MergeSort class.

class MergeSort(object):

    def __init__(self, array):
        self.array = array

    def merge_sort(self):
        if len(self.array) > 1:
            m = len(self.array)//2
            left = self.array[:m]
            right = self.array[m:]

            leftsorter = MergeSort(left)
            leftsorter.merge_sort()
            rightsorter = MergeSort(right)
            rightsorter.merge_sort()

            i = 0
            j = 0
            k = 0

            while i < len(left) and j < len(right):
                if left[i] < right[j]:
                    self.array[k] = left[i]
                    i += 1
                else:
                    self.array[k] = right[j]
                    j += 1
                k += 1

            while i < len(left):
                self.array[k] = left[i]
                i += 1
                k += 1

            while j < len(right):
                self.array[k] = right[j]
                j += 1
                k += 1

x = MergeSort([3,5,6,2,1,4,10,9,8,7])
x.merge_sort()
x.array

Out[ ]: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]

Well done everybody!

score -1 · Answer 3 · answered Jun 08 '18 at 06:34

-1

You need to call self.sort() within the class.

A larger problem is that none of your function or class method return anything, just prints, are you satisfied with that?

answered Jun 08 '18 at 06:34

Evgeny

4,173
2
19
39

1

First, `self.sort` won't work. Second, if the method mutates the list in-place, it doesn't have to (and, in fact, shouldn't) return anything. – abarnert Jun 08 '18 at 06:35
1

@abarnet, the method can work on the instance variable, but why the function just print? print(sort(x)) is much more explicit – Evgeny Jun 08 '18 at 06:42
1

Calling `self.sort()` will just restart sorting the same list you're in the middle of sorting, which will just lead to infinite recursion. It won't sort the left half, which is what the OP is trying to do. I agree that the `print` is not a great design, but it is useful for debugging purposes, so maybe that's why he did it? But `print(sort(x))` is not good either. In Python, functions that mutate objects in-place usually don't return those objects. Just like `list.sort()` doesn't return the sorted `self`, it's idiomatic for `MergeSort.sort()` to not return _its_ sorted `self.array`. – abarnert Jun 08 '18 at 06:44
1

@abarnert, even for debugging purposes, the function is rather meaningless, if the sorted array is not returned. I do not understand what you mean by the 'function that mutate objects inplace', can you give an example? That may be a class method, but rather pointless for a standalone sort function. When added a return value, the original sort gives wrong recursion, that is probably why the OP seeks a class, but rather wrongly. btw, print(sort(x)) is proper direction for this function, considering you would not call list.sort() a function, but rather a method. – Evgeny Jun 08 '18 at 06:56
1

I don't know how else to explain "mutates in-place". Maybe try running the OP's (working, non-class-version) code, and doing a `print(arr)` after `sort(arr)` and it'll be clear to you. – abarnert Jun 08 '18 at 07:24
Also, the original sort doesn't do the recursion wrong. As can be demonstrated by the fact that it does in fact sort the list correctly, and in log-linear time. – abarnert Jun 08 '18 at 07:25
@abarnet, I'm comfortable with inline mutation, just confused by the fact a user cannot get sorted_list = ... from neither example, especially the class. – Evgeny Jun 08 '18 at 07:31
if you call `x.array` after instantiating an object and calling sort on it, then yes you do get a sorted list returned to you. – reka18 Jun 08 '18 at 08:30

Implementing merge sort function in python class, errors

3 Answers3

Linked