Create graphs/dijkstra_alternate.py by AtulRajput01 · Pull Request #7405 · TheAlgorithms/Python

AtulRajput01 · 2022-10-18T17:04:11Z

Describe your change:

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

Checklist:

for more information, see https://pre-commit.ci

AtulRajput01

all set , all functions work well

graphs/dijkstra.py

ZeroDayOwl · 2022-10-19T10:29:00Z

@AtulRajput01 check pre-commit details and fix the issues.

cclauss · 2022-10-19T11:34:08Z

Instead of deleting someone else's work, let's move this to a separate file graphs/dijkstra_alternate.py and make sure that naming, tests, type hints are in compliance with https://github.com/TheAlgorithms/Python/blob/master/CONTRIBUTING.md

AtulRajput01 · 2022-10-19T11:59:45Z

Instead of deleting someone else's work, let's move this to a separate file graphs/dijkstra_alternate.py and make sure that naming, tests, type hints are in compliance with https://github.com/TheAlgorithms/Python/blob/master/CONTRIBUTING.md

okay

AtulRajput01

done

AtulRajput01 · 2022-10-19T16:54:08Z

@AtulRajput01 check pre-commit details and fix the issues.

code fix .

cclauss · 2022-10-19T17:19:36Z

graphs/dijkstra.py:6:10: N802 function name 'printSolution' should be lowercase
graphs/dijkstra.py:11:10: N802 function name 'minDistance' should be lowercase
graphs/dijkstra.py:11:34: N803 argument name 'sptSet' should be lowercase
graphs/dijkstra.py:13:9: A001 variable "min" is shadowing a python builtin
graphs/dijkstra.py:16:44: E712 comparison to False should be 'if cond is False:' or 'if not cond:'
graphs/dijkstra.py:17:17: A001 variable "min" is shadowing a python builtin
graphs/dijkstra.py:26:10: N806 variable 'sptSet' in function should be lowercase
graphs/dijkstra.py:28:13: B007 Loop control variable 'cout' not used within the loop body. If this is intended, start the name with an underscore.
graphs/dijkstra.py:36:35: E712 comparison to False should be 'if cond is False:' or 'if not cond:'

AtulRajput01 · 2022-10-19T18:04:49Z

done sir

cclauss

According to CONTRBUTING.md there needs to be at least one algorithmic function that takes in one or more parameters and returns one or more values. Algorithmic functions should not print().

graphs/dijkstra.py

Co-authored-by: Christian Clauss <cclauss@me.com>

AtulRajput01 · 2022-10-19T18:41:58Z

According to CONTRBUTING.md there needs to be at least one algorithmic function that takes in one or more parameters and returns one or more values. Algorithmic functions should not print().

okay sir ,i understood very well.

Abinashbunty

In line 30, during function call, it should be self.minimum_distance
All dist should be distances ~~or vice versa~~.
All sptset should be spt_set ~~or vice versa~~ or a more self-documenting name.
printsolution should be print_solution ~~or vice versa.~~
Include doctests.
Provide some comments in algorithm execution.

Abinashbunty · 2022-10-19T19:51:30Z

A general advice is to execute the code online/locally to fix the compilation(interpreter) errors. 👍🏻

for more information, see https://pre-commit.ci

AtulRajput01 · 2022-10-19T20:26:49Z

A general advice is to execute the code online/locally to fix the compilation(interpreter) errors. 👍🏻

it's working on my machine as well, then why pre-commit failure accur

cclauss · 2022-10-19T20:31:09Z

graphs/dijkstra.py:22:48: E712 comparison to False should be 'if cond is False:' or 'if not cond:'
graphs/dijkstra.py:37:13: B007 Loop control variable 'cout' not used within the loop body. If this is intended, start the name with an underscore.
graphs/dijkstra.py:55:35: E712 comparison to False should be 'if cond is False:' or 'if not cond:'

AtulRajput01

+1

Abinashbunty

Moreover, check my comment above which Christian has edited. Use distances, spt_set and print_solution.

graphs/dijkstra.py

for more information, see https://pre-commit.ci

cclauss · 2022-10-19T21:56:19Z

OK. I changed the filename, used self-documenting names for functions and variables, added type hints, and docstrings with doctests.

I still have no clue what a better name for sptset would be except perhaps visited.

Abinashbunty · 2022-10-19T22:10:10Z

@cclauss visited seems the only logical variable name for sptset.

graphs/dijkstra_alternate.py

ZeroDayOwl · 2022-10-20T03:45:37Z

A general advice is to execute the code online/locally to fix the compilation(interpreter) errors. 👍🏻

it's working on my machine as well, then why pre-commit failure accur

Did you run the tests as mentioned in the contributing guideline?

AtulRajput01 · 2022-10-20T07:42:05Z

ok

…

On Thu, Oct 20, 2022 at 9:15 AM Paul ***@***.***> wrote: A general advice is to execute the code online/locally to fix the compilation(interpreter) errors. 👍🏻 it's working on my machine as well, then why pre-commit failure accur Did you run the tests as mentioned in the contributing guideline? — Reply to this email directly, view it on GitHub <#7405 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AWC54XNGECL7KXCONOD75ZDWEC57DANCNFSM6AAAAAARIJNPRI> . You are receiving this because you were mentioned.Message ID: ***@***.***>

AtulRajput01 and others added 2 commits October 18, 2022 22:33

Update dijkstra.py

e50b975

[pre-commit.ci] auto fixes from pre-commit.com hooks

74c2f79

for more information, see https://pre-commit.ci

AtulRajput01 commented Oct 18, 2022

View reviewed changes

graphs/dijkstra.py Outdated Show resolved Hide resolved

AtulRajput01 commented Oct 19, 2022

View reviewed changes

Update dijkstra.py

0a17d6a

algorithms-keeper bot added the awaiting reviews This PR is ready to be reviewed label Oct 19, 2022

cclauss requested changes Oct 19, 2022

View reviewed changes

graphs/dijkstra.py Outdated Show resolved Hide resolved

graphs/dijkstra.py Outdated Show resolved Hide resolved

graphs/dijkstra.py Outdated Show resolved Hide resolved

algorithms-keeper bot added awaiting changes A maintainer has requested changes to this PR and removed awaiting reviews This PR is ready to be reviewed labels Oct 19, 2022

Update graphs/dijkstra.py

52ae3d6

Co-authored-by: Christian Clauss <cclauss@me.com>

algorithms-keeper bot added awaiting reviews This PR is ready to be reviewed and removed awaiting changes A maintainer has requested changes to this PR labels Oct 19, 2022

AtulRajput01 and others added 2 commits October 20, 2022 00:08

Update graphs/dijkstra.py

60886b5

Co-authored-by: Christian Clauss <cclauss@me.com>

Update graphs/dijkstra.py

1a5eff0

Co-authored-by: Christian Clauss <cclauss@me.com>

algorithms-keeper bot added the tests are failing Do not merge until tests pass label Oct 19, 2022

Abinashbunty suggested changes Oct 19, 2022

View reviewed changes

AtulRajput01 and others added 2 commits October 20, 2022 01:52

Update dijkstra.py

3f2b017

[pre-commit.ci] auto fixes from pre-commit.com hooks

d3dc21c

for more information, see https://pre-commit.ci

algorithms-keeper bot removed the tests are failing Do not merge until tests pass label Oct 19, 2022

Update dijkstra.py

0b28bf9

AtulRajput01 commented Oct 19, 2022

View reviewed changes

Abinashbunty suggested changes Oct 19, 2022

View reviewed changes

graphs/dijkstra.py Outdated Show resolved Hide resolved

graphs/dijkstra.py Outdated Show resolved Hide resolved

cclauss requested changes Oct 19, 2022

View reviewed changes

graphs/dijkstra.py Outdated Show resolved Hide resolved

graphs/dijkstra.py Outdated Show resolved Hide resolved

graphs/dijkstra.py Outdated Show resolved Hide resolved

algorithms-keeper bot added awaiting changes A maintainer has requested changes to this PR and removed awaiting reviews This PR is ready to be reviewed labels Oct 19, 2022

Apply suggestions from code review

1efce83

algorithms-keeper bot added awaiting reviews This PR is ready to be reviewed and removed awaiting changes A maintainer has requested changes to this PR labels Oct 19, 2022

cclauss and others added 3 commits October 19, 2022 23:52

Create dijkstra_alternate.py

f7c8139

Update dijkstra.py

ce2e683

[pre-commit.ci] auto fixes from pre-commit.com hooks

2e26c75

for more information, see https://pre-commit.ci

int(1e7)

47d405c

algorithms-keeper bot added the tests are failing Do not merge until tests pass label Oct 19, 2022

Update dijkstra_alternate.py

8890161

algorithms-keeper bot removed the tests are failing Do not merge until tests pass label Oct 19, 2022

cclauss reviewed Oct 19, 2022

View reviewed changes

graphs/dijkstra_alternate.py Outdated Show resolved Hide resolved

Update graphs/dijkstra_alternate.py

78ab169

cclauss changed the title ~~Update dijkstra.py~~ Create graphs/dijkstra_alternate.py Oct 19, 2022

AtulRajput01 changed the title ~~Create graphs/dijkstra_alternate.py~~ Create graphs/dijkstra_alternate.py Oct 20, 2022

sptset --> visited

fa6d2ed

cclauss approved these changes Oct 20, 2022

View reviewed changes

algorithms-keeper bot added awaiting reviews This PR is ready to be reviewed and removed awaiting reviews This PR is ready to be reviewed labels Oct 20, 2022

cclauss merged commit 4829fea into TheAlgorithms:master Oct 20, 2022

algorithms-keeper bot removed the awaiting reviews This PR is ready to be reviewed label Oct 20, 2022

Uh oh!

Conversation

AtulRajput01 commented Oct 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Describe your change:

Checklist:

Uh oh!

AtulRajput01 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ZeroDayOwl commented Oct 19, 2022

Uh oh!

cclauss commented Oct 19, 2022

Uh oh!

AtulRajput01 commented Oct 19, 2022

Uh oh!

AtulRajput01 left a comment • 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!

AtulRajput01 commented Oct 19, 2022

Uh oh!

cclauss commented Oct 19, 2022

Uh oh!

AtulRajput01 commented Oct 19, 2022 • edited by cclauss Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cclauss left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AtulRajput01 commented Oct 19, 2022

Uh oh!

Abinashbunty left a comment • edited by cclauss Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Abinashbunty commented Oct 19, 2022

Uh oh!

AtulRajput01 commented Oct 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cclauss commented Oct 19, 2022

Uh oh!

AtulRajput01 left a comment

Choose a reason for hiding this comment

Uh oh!

Abinashbunty left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cclauss commented Oct 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Abinashbunty commented Oct 19, 2022

Uh oh!

Uh oh!

ZeroDayOwl commented Oct 20, 2022

Uh oh!

AtulRajput01 commented Oct 20, 2022 via email

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

AtulRajput01 commented Oct 18, 2022 •

edited

Loading

AtulRajput01 left a comment •

edited

Loading

AtulRajput01 commented Oct 19, 2022 •

edited by cclauss

Loading

Abinashbunty left a comment •

edited by cclauss

Loading

AtulRajput01 commented Oct 19, 2022 •

edited

Loading

cclauss commented Oct 19, 2022 •

edited

Loading