Python implementation for Jarvis March #223

wmboyles · 2018-07-06T05:06:51Z

I struggled a bit, so some of the individual commits are unnecessary.

Added python3 example for Jarvits March.

Create jarvits.py

jiegillet · 2018-07-07T00:58:43Z

chapters/computational_geometry/gift_wrapping/jarvis_march/code/py/jarvits.py

+
+
+#Find the leftmost point in the list
+def leftmost(gift):


You can use a key= with min to reduce this function to one line: min(gift, key=lambda point: point[0])

Well, actually, in this case you can just use min(gift) directly, because when comparing a list, python compares by the first element first.

jiegillet · 2018-07-07T00:59:26Z

chapters/computational_geometry/gift_wrapping/jarvis_march/code/py/jarvits.py

+
+
+#Are two points the same point?
+def equal(p1,p2):


Can't you simply use p1==p2?

jiegillet · 2018-07-07T01:01:35Z

chapters/computational_geometry/gift_wrapping/jarvis_march/code/py/jarvits.py

+
+def jarvisMarch(gift):
+    n = len(gift)  #Number of points in list
+    hull = [(None,None)] * n #The hull, initially empty points


I know that initializing lists is a good idea in general, but here it looks clunky (especially later when you remove the empty points). I would just append points to an initially empty hull as they come.

jiegillet · 2018-07-07T01:05:40Z

hi @wmboyles, welcome to the AAA and thanks for your contribution.
On top of my other comments in text, could you add a main() function that actually runs on a set of points?

wmboyles · 2018-07-07T03:41:06Z

Thanks for the input @jiegillet . I have implemented everything you asked for. Do you think the hull should include the starting point both at the beginning and end of the hull? I think having it emphasizes the fact that the convex hull is a closed shape, but understand the possible confusion of having the same point in the hull twice.

jiegillet · 2018-07-07T04:41:55Z

chapters/computational_geometry/gift_wrapping/jarvis_march/code/py/jarvits.py

+def jarvisMarch(gift):
+    n = len(gift)  #Number of points in list
+    pointOnHull = min(gift) #leftmost point in gift
+    hull = [pointOnHull] #leftmost point guranteed to be in hull


tiny typo guranteed

jiegillet · 2018-07-07T04:43:07Z

chapters/computational_geometry/gift_wrapping/jarvis_march/code/py/jarvits.py

+    while True:
+        endpoint = gift[0]
+        for j in range(1,n):
+            if endpoint==pointOnHull or not CCW(gift[j],hull[i],endpoint):


hull[i] is the only point where you use i, correct? Could you get away with using hull[-1] and drop i altogether?

jiegillet · 2018-07-07T04:49:31Z

Thanks for the quick answer! Your changes are great.

I think the starting point should only be there once. We don't need to emphasize that a hull is a closed shape, it's the definition.
I left a couple of small comments.
Could you remove your commit on the Euclidian Algorithm? It gets really messy when a PR has more than one topic. Feel free to open a new PR about that if you want.
You need to in include your file into chapters/computational_geometry/gift_wrapping/jarvis_march/jarvis_march.md. You can mimic how the other languages do it.
Small detail, but your file name is jarvits.py. Could you change it to jarvis.py or jarvisMarch.py?
Is it your first contribution? If so you can add you name into CONTRIBUTORS.md (at the root of the repo)

wmboyles · 2018-07-07T15:42:14Z

Changes made. I added myself as a contributor and changed the .md file in a separate pull requests.

jiegillet · 2018-07-08T00:20:43Z

It's almost done, but during your PR, the euclidian algorithm has changed, and here you are still modifying it. That's exactly why you shouldn't mix PRs :)
Get rid of any change toeuclidean_example.py and I'll merge directly.

Edit: also add the changes to jarvis_march.md and CONTRIBUTORS.md here.

wmboyles · 2018-07-08T01:00:37Z

Hopefully that should fix all of my mistakes.

jiegillet

Looks good, thanks a lot!

wmboyles added 5 commits July 5, 2018 23:52

Create jarvits.py

84b4040

Added python3 example for Jarvits March.

Create Python

01c7af9

Delete Python

1ce66fa

Merge pull request #1 from wmboyles/wmboyles-patch-1

692644a

Create jarvits.py

Updates jarvis.py with better variable name

78db02c

Gathros added the Implementation This provides an implementation for an algorithm. (Code and maybe md files are edited.) label Jul 6, 2018

jiegillet reviewed Jul 7, 2018

View reviewed changes

jiegillet self-assigned this Jul 7, 2018

wmboyles added 2 commits July 6, 2018 23:33

Update jarvits.py

973e4c3

Update jarvits.py

3b143a2

Added recursive Euclidean Algorithm

8522fe8

jiegillet reviewed Jul 7, 2018

View reviewed changes

wmboyles added 2 commits July 7, 2018 11:19

Merge branch 'master' into master

13103a9

Update and rename jarvits.py to jarvisMarch.py

1339f92

lulucca12 mentioned this pull request Jul 7, 2018

Update .md to include Python code #230

Closed

jiegillet mentioned this pull request Jul 8, 2018

Added myself as a contributor #229

Closed

wmboyles added 3 commits July 7, 2018 20:54

euclidean_example.py now matches master branch

a3c9463

Added my name

929c0f7

Added python to .md

eed06c6

jiegillet approved these changes Jul 8, 2018

View reviewed changes

jiegillet merged commit b1d367c into algorithm-archivists:master Jul 8, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Python implementation for Jarvis March #223

Python implementation for Jarvis March #223

Uh oh!

wmboyles commented Jul 6, 2018

Uh oh!

jiegillet Jul 7, 2018

Uh oh!

jiegillet Jul 7, 2018

Uh oh!

jiegillet Jul 7, 2018

Uh oh!

jiegillet Jul 7, 2018 •

edited

Loading

Uh oh!

jiegillet commented Jul 7, 2018

Uh oh!

wmboyles commented Jul 7, 2018

Uh oh!

jiegillet Jul 7, 2018

Uh oh!

jiegillet Jul 7, 2018 •

edited

Loading

Uh oh!

jiegillet commented Jul 7, 2018 •

edited

Loading

Uh oh!

wmboyles commented Jul 7, 2018 •

edited

Loading

Uh oh!

jiegillet commented Jul 8, 2018 •

edited

Loading

Uh oh!

wmboyles commented Jul 8, 2018

Uh oh!

jiegillet left a comment

Uh oh!

Uh oh!

Uh oh!

Python implementation for Jarvis March #223

Python implementation for Jarvis March #223

Uh oh!

Conversation

wmboyles commented Jul 6, 2018

Uh oh!

jiegillet Jul 7, 2018

Choose a reason for hiding this comment

Uh oh!

jiegillet Jul 7, 2018

Choose a reason for hiding this comment

Uh oh!

jiegillet Jul 7, 2018

Choose a reason for hiding this comment

Uh oh!

jiegillet Jul 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jiegillet commented Jul 7, 2018

Uh oh!

wmboyles commented Jul 7, 2018

Uh oh!

jiegillet Jul 7, 2018

Choose a reason for hiding this comment

Uh oh!

jiegillet Jul 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jiegillet commented Jul 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wmboyles commented Jul 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jiegillet commented Jul 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wmboyles commented Jul 8, 2018

Uh oh!

jiegillet left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jiegillet Jul 7, 2018 •

edited

Loading

jiegillet Jul 7, 2018 •

edited

Loading

jiegillet commented Jul 7, 2018 •

edited

Loading

wmboyles commented Jul 7, 2018 •

edited

Loading

jiegillet commented Jul 8, 2018 •

edited

Loading