Added a stack and queue implementation for python3 #1008

Michae1CC · 2023-02-15T03:58:50Z

Added Stack and Queue implementations for python3

Motivation

Stack and Queue implementations for python3 are currently not present on the website.
Current pull requests for this feature have gone cold.

Testing

Ran npm run serve and manually confirmed changes.

Notes

I've added the 'interfaces' to this implementation to more closely match the Java and Typescript implementations.

Michae1CC · 2023-02-15T04:22:01Z

@Amaras @leios would either of you mind reviewing this? Thanks.

Amaras

Sorry about the delay, I was not in the mental space to review this PR before...
Thanks for your interest for the Arcane Algorithm Archive!

So, onto the review, I tend to make harsh code reviews, so sorry in advance if I'm too harsh on you.

It seems you have basically transpiled Java or TypeScript code into Python, so it's definitely not idiomatic, especially with the naming conventions.

I'm definitely not a fan of using abstract base classes when it's not necessary to do so, and you have not proven the need to use them, so that's a hard no for me.
If you had multiple implementations, then maybe I would judge an ABC more favourably, but it is not the case here, so please remove them both.

Amaras · 2023-03-21T09:59:40Z

contents/stacks_and_queues/code/python/Stack.py

+class IStack(Generic[T], Sized, abc.ABC):
+    @abc.abstractmethod
+    def pop(self) -> T:
+        """
+        Removes the last element from the stack and returns it.
+        """
+        ...
+
+    @abc.abstractmethod
+    def push(self, element: T) -> int:
+        """
+        Adds an element to the end of the stack and returns the
+        new length of the stack.
+        """
+        ...
+
+    @abc.abstractmethod
+    def top(self) -> T:
+        """
+        Returns the first element of the stack.
+        """
+        ...


I feel that this is not necessary for understanding, and may actively harm understanding, as interfaces are not really that common in Python code, contrary to (say) Java code.

Yes I agree with this, like I mentioned below this is my first time contributing and I just didn't know what was expected. I've removed both the abcs in the new revision.

Amaras · 2023-03-21T10:02:07Z

contents/stacks_and_queues/code/python/queue.py

+class IQueue(Generic[T], Sized, abc.ABC):
+    @abc.abstractmethod
+    def dequeue(self) -> T:
+        """
+        Removes the first element from the queue and returns it.
+        """
+        ...
+
+    @abc.abstractmethod
+    def enqueue(self, element: T) -> int:
+        """
+        Add an element at the end of the queue and returns the
+        new size of the queue.
+        """
+        ...
+
+    @abc.abstractmethod
+    def front(self) -> T:
+        """
+        Returns the first element in the queue without removing it.
+        """
+        ...


Same thing as the stack one: not sure whether this interface is necessary.

Amaras · 2023-03-21T10:03:51Z

contents/stacks_and_queues/stacks_and_queues.md

+In _stacks_, data follows _Last In, First Out_ (LIFO), which basically means that whichever element you put in last will be the first element you take out. It acts exactly like a stack in real life. If you put a book on a stack of other books, the first book you will look at when sifting through the stack will be the book you just put on the stack.

-In *Queues*, data follows *First In, First Out* (FIFO), which means that whichever element you put in first will be the first element you take out. Imagine a queue of people. It would be unfair if the first person in line for groceries were not the first person to receive attention once the attendant finally shows up.
+In _Queues_, data follows _First In, First Out_ (FIFO), which means that whichever element you put in first will be the first element you take out. Imagine a queue of people. It would be unfair if the first person in line for groceries were not the first person to receive attention once the attendant finally shows up.


Was changing the formatting necessary?

I've reverted this in the new commit

Michae1CC · 2023-03-29T13:36:05Z

Hi @Amaras , thanks so much for taking a look at this PR! Apologies for the the late reply, I was out of town for a little while. This is my first time contributing to the Algorithm Archive so I just wasn't 100% what was expected. I've attempted addressed all of your previous comments in a new revision. Please let me know if there's anything else you would like me to change.

Amaras

Don't worry about being late, I'm not that much on GitHub anyway.
I think this revision is much better than the previous one.
We just need to ensure we're on the same page concerning the use of the Sized abstract base class, so feel free to remove it or argue for the inclusion, whichever you prefer.

We're definitely doing a last pass once this point is resolved :)

Amaras · 2023-03-29T15:18:15Z

contents/stacks_and_queues/code/python/queue.py

+T = TypeVar("T")
+
+
+class Queue(Generic[T], Sized):


Since you define __len__, I'm not sure if you need to use the Sized abc.

Good point. I think I originally added in the Sized abc to save myself typing out __len__ in the 'interfaces' I had before. But since the interfaces are gone now, there's really no reason to keep them around and have been removed in the latest commit.

Amaras · 2023-03-29T15:18:54Z

contents/stacks_and_queues/code/python/stack.py

+T = TypeVar("T")
+
+
+class Stack(Generic[T], Sized):


Same as with the Queue class, not sure if you need to use the Sized abc.

Michae1CC · 2023-03-30T05:27:05Z

Thanks for the quick response! I've attempted to address these new comments in the latest commit. Again, let me know if there is anything else you would like me to change.

Amaras · 2023-03-30T09:37:32Z

Looks good to me.
You'll just need to merge main into your branch and I'll start the CI check.
If that passes, I'll merge your PR.

Michae1CC · 2023-03-30T13:28:37Z

Done! Rebased and added changes to my main branch and pushed them to my forked repo.

Amaras

LGTM

Michae1CC · 2023-03-30T13:39:48Z

Thanks again for helping out @Amaras , really appreciate your time/effort! You may just want to close this duplicate PR that went cold: #771 .

Amaras self-requested a review March 21, 2023 09:54

Amaras requested changes Mar 21, 2023

View reviewed changes

Michae1CC force-pushed the stacks_and_queues_in_python branch from ba84348 to 7eb2445 Compare March 29, 2023 13:29

Amaras reviewed Mar 29, 2023

View reviewed changes

Added a stack and queue implementation for python3

ee24f01

Michae1CC force-pushed the stacks_and_queues_in_python branch from f496493 to ee24f01 Compare March 30, 2023 13:24

Amaras approved these changes Mar 30, 2023

View reviewed changes

leios merged commit 5aabe09 into algorithm-archivists:main Mar 30, 2023

Uh oh!

Added a stack and queue implementation for python3 #1008

Added a stack and queue implementation for python3 #1008

Uh oh!

Conversation

Michae1CC commented Feb 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Added Stack and Queue implementations for python3

Motivation

Testing

Notes

Uh oh!

Michae1CC commented Feb 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Amaras left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Michae1CC commented Mar 29, 2023

Uh oh!

Amaras left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Michae1CC commented Mar 30, 2023

Uh oh!

Amaras commented Mar 30, 2023

Uh oh!

Michae1CC commented Mar 30, 2023

Uh oh!

Amaras left a comment

Choose a reason for hiding this comment

Uh oh!

Michae1CC commented Mar 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Michae1CC commented Feb 15, 2023 •

edited

Loading

Michae1CC commented Feb 15, 2023 •

edited

Loading

Michae1CC commented Mar 30, 2023 •

edited

Loading