iframe-proxy

ghost · 2021-02-13T02:55:10Z

Describe your change:

Add an algorithm?
Fix a bug or typo in an existing algorithm?
Documentation change?

Checklist:

ghost

Click here to look at the relevant links ⬇️

🔗 Relevant Links

Repository:

Contributing guidelines

Project Euler solution guidelines

Python:

Formatted string literals (f-strings)

Type hints

doctest

unittest

pytest

Automated review generated by algorithms-keeper. If there's any problem regarding this review, please open an issue about it.

algorithms-keeper commands and options

algorithms-keeper actions can be triggered by commenting on this PR:

@algorithms-keeper review to trigger the checks for only added pull request files

@algorithms-keeper review-all to trigger the checks for all the pull request files, including the modified files. As we cannot post review comments on lines not part of the diff, this command will post all the messages in one comment.

NOTE: Commands are in beta and so this feature is restricted only to a member or owner of the organization.

ghost · 2021-02-13T02:55:23Z

+    # remove the lowest and the most left point from points for preparing for sort
+    points.pop(minidx)
+
+    def angle_comparer(p: list[int, int]) -> float:


As there is no test file in this pull request nor any test function or class in the file other/graham_scan.py, please provide doctest for the function angle_comparer

Please provide descriptive name for the parameter: p

To reviewers:
This is an inner function.

argument name is fixed here fdaacde

ghost · 2021-02-13T02:55:24Z

+        angle = degrees(atan2(y - miny, x - minx))
+        return angle
+
+    sorted_points = sorted(points, key=lambda p: angle_comparer(p))


Please provide descriptive name for the parameter: p

argument name is fixed here fdaacde

ghost · 2021-02-13T02:55:24Z

+        straight = 2
+        right = 3
+
+    def check_direction(


As there is no test file in this pull request nor any test function or class in the file other/graham_scan.py, please provide doctest for the function check_direction

To reviewers:
This is an inner function

ghost

Click here to look at the relevant links ⬇️

🔗 Relevant Links

Repository:

Contributing guidelines

Project Euler solution guidelines

Python:

Formatted string literals (f-strings)

Type hints

doctest

unittest

pytest

Automated review generated by algorithms-keeper. If there's any problem regarding this review, please open an issue about it.

algorithms-keeper commands and options

algorithms-keeper actions can be triggered by commenting on this PR:

@algorithms-keeper review to trigger the checks for only added pull request files

@algorithms-keeper review-all to trigger the checks for all the pull request files, including the modified files. As we cannot post review comments on lines not part of the diff, this command will post all the messages in one comment.

NOTE: Commands are in beta and so this feature is restricted only to a member or owner of the organization.

ghost · 2021-02-13T02:58:44Z

+    # remove the lowest and the most left point from points for preparing for sort
+    points.pop(minidx)
+
+    def angle_comparer(point: list[int, int]) -> float:


As there is no test file in this pull request nor any test function or class in the file other/graham_scan.py, please provide doctest for the function angle_comparer

ghost · 2021-02-13T02:58:44Z

+        straight = 2
+        right = 3
+
+    def check_direction(


As there is no test file in this pull request nor any test function or class in the file other/graham_scan.py, please provide doctest for the function check_direction

ghost · 2021-02-13T03:06:56Z

To reviewers:

The test is failed at pre-commit because CI executes black for whole files in this repository, and it changes 814 files.

I did black with its newest version, and so CI does. It should be updated, or ignore this fail.

…dd_graham_scan

…add_graham_scan

cclauss · 2021-03-02T12:10:10Z

Doesn't this algorithm belong in https://github.com/TheAlgorithms/Python/tree/master/sorts ? or graphs?

How does this algorithm compare with
https://github.com/TheAlgorithms/Python/blob/master/divide_and_conquer/convex_hull.py

ghost · 2021-03-02T12:14:01Z

@cclauss
Hi. I'm not sure if this algorithm should belong to sorts, because this aims to find the convex hull of a finite set of points. It's kind of coordinate things rather than sorting.

cclauss · 2021-03-02T12:15:36Z

Putting algorithms in other makes them very hard for visitors to find. Is there another category that makes sense?

ghost · 2021-03-02T12:26:00Z

@cclauss

I see. searches must be fitting. I will move this.

How does this algorithm compare with master/divide_and_conquer/convex_hull.py

divide_and_conquer algorithm is also known well to solve convex hull problem only by O(n * logn) time complexity. However, it's approach is totally different from graham scan.

Graham scan doesn't use divide_and_conquer approach, but it focuses on sorting points by angle.
We'll look for all points at same time and trace the periphery with checking if the angle is towards to inner or outer.

ghost · 2021-03-02T12:27:22Z

Oh. Thank you for merging.

I wonder if I should create other PR to move this to searches, or shouldn't....

cclauss · 2021-03-02T12:35:13Z

* Add graham scan algorithm * Fix argument name p with point * Add tests in inner function * updating DIRECTORY.md * Fix graham scan for isort --profile=black Co-authored-by: github-actions <${GITHUB_ACTOR}@users.noreply.github.com>

Add graham scan algorithm

2563270

ghost added require descriptive names This PR needs descriptive function and/or variable names require tests Tests [doctest/unittest/pytest] are required awaiting reviews This PR is ready to be reviewed labels Feb 13, 2021

ghost reviewed Feb 13, 2021

View reviewed changes

ghost added the tests are failing Do not merge until tests pass label Feb 13, 2021

Fix argument name p with point

fdaacde

ghost removed the require descriptive names This PR needs descriptive function and/or variable names label Feb 13, 2021

ghost reviewed Feb 13, 2021

View reviewed changes

ulwlu added 2 commits February 22, 2021 12:46

Merge branch 'master' of git://github.com/TheAlgorithms/Python into a…

efdca9e

…dd_graham_scan

Add tests in inner function

79c4b6a

ghost removed the require tests Tests [doctest/unittest/pytest] are required label Feb 22, 2021

github-actions and others added 3 commits February 22, 2021 04:08

updating DIRECTORY.md

bc2b60d

Fix graham scan for isort --profile=black

c232984

Merge branch 'add_graham_scan' of ssh://github.com/ulwlu/Python into …

ecd59a1

…add_graham_scan

ghost removed the tests are failing Do not merge until tests pass label Mar 2, 2021

ghost closed this Mar 2, 2021

ghost reopened this Mar 2, 2021

cclauss closed this Mar 2, 2021

cclauss reopened this Mar 2, 2021

cclauss approved these changes Mar 2, 2021

View reviewed changes

ghost removed the awaiting reviews This PR is ready to be reviewed label Mar 2, 2021

cclauss merged commit a796ccf into TheAlgorithms:master Mar 2, 2021

Sunbelt Computer Software

PL/B Language Development and Support

Uh oh!

Uh oh!

Conversation

ghost commented Feb 13, 2021

Describe your change:

Checklist:

Uh oh!

ghost left a comment

Choose a reason for hiding this comment

🔗 Relevant Links

Repository:

Python:

Automated review generated by algorithms-keeper. If there's any problem regarding this review, please open an issue about it.

algorithms-keeper actions can be triggered by commenting on this PR:

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!

ghost left a comment

Choose a reason for hiding this comment

🔗 Relevant Links

Repository:

Python:

Automated review generated by algorithms-keeper. If there's any problem regarding this review, please open an issue about it.

algorithms-keeper actions can be triggered by commenting on this PR:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost commented Feb 13, 2021

Uh oh!

cclauss commented Mar 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented Mar 2, 2021

Uh oh!

cclauss commented Mar 2, 2021

Uh oh!

ghost commented Mar 2, 2021

Uh oh!

ghost commented Mar 2, 2021

Uh oh!

cclauss commented Mar 2, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

cclauss commented Mar 2, 2021 •

edited

Loading