Python3 Stack implementation with List built-in

Question

So, inspired by this other code review, I wanted to see if I could implement a very basic stack without the linked-list approach taken by the OP in their implementation.

To that end, I came up with the following Stack class. I wrote it for Python 3, but it actually works as-is in Python 2 as well:

class Stack:
 _stack = []
 def __init__(self):
 self._stack = []
 @property
 def size(self):
 return len(self._stack)
 @property
 def count(self):
 return self.size
 def __sizeof__(self):
 return self.size
 def pop(self):
 if self.size == 0:
 return "Cannot pop from an empty stack."
 item = self._stack[self.size - 1]
 self._stack.remove(item)
 return item
 def peek(self):
 if self.size == 0:
 return "Cannot peek into an empty stack."
 return self._stack[self.size - 1]
 def push(self, item):
 self._stack.append(item)
 def __iter__(self):
 return iter(self._stack)

I'm probably missing some key parts of what one would want from a Stack, but any improvement is welcome. Literally any improvement.

Note there were a couple things I did intentionally:

1. I intentionally initialize the _stack attribute as an empty list both in the class itself and in __init__. For some reason PyCharm complains if the attribute isn't already part of the class when being worked with in __init__, so this is more or less to get PyCharm to stop yelling at me.
2. I intentionally have __sizeof__ declared as referring to the size property function. This lets len(StackObject) work if someone doesn't want to do stack.size.

With these in mind, feel free to tell me what you think, or point out any mistakes I've made. (I'm not an expert in data structures, so I may have gotten some points messed up a little).

• aghast: __sizeof__ is expected to return memory size rather then length. I think you meant to implement __len__ instead?
• Thomas Ward: @AustinHastings addressed by Coal_'s answer, and admittedly a confusion of the special names on my part. Properly fixed in the reviewed/altered code I'm going to put into place shortly via another review.
• Thomas Ward: Round #2 of the reviews has been posted for your consideration :)
• WorldSEnder: self._stack[self.size - 1] could be simplified to self._stack[-1]
• Buge: count seems to not fit in with other python structures such as list and str which have a count function which counts the number of times a specific value appears.

Answer 1

By far the biggest problem this code has is error handling. When popping or peeking from an empty stack this returns a string. this is really broken. For an example of why, suppose I add the string "Cannot peek into an empty stack." to the stack and then try to see what the top element is. I won't know if the answer I got was because the stack was empty, or because that string was in the stack. The solution to this is to raise an IndexError when the stack is empty. I also think that you shouldn't define either size or count, as the pythonic way of getting somethings length is to call len(x). You might also be interested to know that python lists have a pop function already, so you could just use that for your pop

• Thomas Ward: I've replaced the string return with a raise to raise an IndexError with a descriptive message. Which is valid Python. Didn't know the list had a pop function, which is nice to know about. That helps reduce the pop function, though I kept the size check in there. Note that other functions rely on size and while I could switch it to a len(...) call it seems like code repetition / annoyingness. Even if I made it an internal item, someone may want to know the size of the stack using old-style calls (like the OP from the post that inspired my implementation, mind you). ...
• Thomas Ward: ... but know that I do agree with you 100% though :)
• user87373: @ThomasWard How is len(self) worse than self.size? It's exactly the same length, plus it's more Pythonic. There should be one-- and preferably only one --obvious way to do it. Having multiple methods and properties that do the same thing is useless code repetition (made worse by the fact that __sizeof__ is not consistent with its intended use).
• Oscar Smith: python made the decision to make len a function rather than to use a size method. This is the api that all builtin's use, and as such is the api you should use if you are writing a library for python code.

Answer 2

The implementation looks pretty clean and simple to me, but here's what caught my attention:

• I think you should sacrifice PyCharm complaining about accessing attributes in __init__ (which is very weird) for readability here. It's just one line of code, but it's pretty confusing since _stack is obviously bound to an instance.

• obj.__sizeof__ != obj.__len__, which is really what you're supposed to override for len(obj) and sys.getsizeof(obj) to work as expected.

  Try this:

  >>> class Foo:
  ... def __init__(self, x):
  ... self.x = x
  ... def __sizeof__(self):
  ... return 100
  ... def __len__(self):
  ... return len(self.x)
  ...
  >>>
  >>> class Bar:
  ... def __init__(self, x):
  ... self.x = x
  ... def __sizeof__(self):
  ... return 200
  ... def __len__(self):
  ... return len(self.x)
  ...
  >>> f = Foo([1,2,3])
  >>> b = Bar([1,2,3,4])
  >>> assert len(f) == 3
  >>> assert len(b) == 4
  

  So far, so good. Calling len on instances of Foo and Bar works just fine. But what happens when we use sys.getsizeof?

  >>> import sys
  >>> sys.getsizeof(f)
  124
  >>> sys.getsizeof(b)
  224
  

  (The added 24 bytes are garbage collector overhead).

• Functions shouldn't return an error message. I would expect pop and seek to return None, or throw an exception. Returning strings is dangerous, especially in this case, where they could be a valid return value, even if nothing went wrong.

Answer 3

I intentionally have __sizeof__ declared as referring to the size property function. This lets len(StackObject) work if someone doesn't want to do stack.size.

From The Zen of Python:

There should be one-- and preferably only one --obvious way to do it.

Python likes there to be One True Way to do things, because it keeps usage consistent everywhere.

In this case, I would do away with the size and count properties entirely, and just define a __len__ (not __sizeof__, like other answers have mentioned). Using __len__ (or rather, the len() function) is the standard way to find the size of a container in Python, so you should provide this method and nothing else.

Instead of:

...
@property
def size(self):
 return len(self._stack)
@property
def count(self):
 return self.size
def __sizeof__(self):
 return self.size
...

Do this:

def __len__(self):
 return len(self._stack)