Skip to content

graphs/kruskal: add a test case to verify the correctness, fix styles - #2443

Merged
poyea merged 7 commits into
TheAlgorithms:masterfrom
meysam81:test/graphs/kruskal
Oct 8, 2020
Merged

graphs/kruskal: add a test case to verify the correctness, fix styles#2443
poyea merged 7 commits into
TheAlgorithms:masterfrom
meysam81:test/graphs/kruskal

Conversation

@meysam81

@meysam81 meysam81 commented Sep 17, 2020

Copy link
Copy Markdown
Contributor

And also to increase the coverage as stated in #2128

Describe your change:

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

Checklist:

  • I have read CONTRIBUTING.md.
  • This pull request is all my own work -- I have not plagiarized.
  • I know that pull requests will not be merged if they fail the automated tests.
  • This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
  • All new Python files are placed inside an existing directory.
  • All filenames are in all lowercase characters with no spaces or dashes.
  • All functions and variable names follow Python naming conventions.
  • All function parameters and return values are annotated with Python type hints.
  • All functions have doctests that pass the automated testing.
  • All new algorithms have a URL in its comments that points to Wikipedia or other similar explanation.
  • If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.

cclauss
cclauss reviewed Sep 17, 2020
View reviewed changes
Comment thread graphs/minimum_spanning_tree_kruskal.py Outdated
cclauss
cclauss reviewed Sep 17, 2020
View reviewed changes
Comment thread graphs/tests/test_min_spanning_tree_kruskal.py Outdated
[3, 5, 14],
[3, 4, 9],
[5, 4, 10],
[1, 7, 11]]

@cclauss cclauss Sep 17, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

psf/black will reformat the matrices.

@poyea poyea self-assigned this Oct 1, 2020
@TravisBuddy

TravisBuddy commented Oct 8, 2020

Copy link
Copy Markdown

Travis tests have failed

Hey @meysam81,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

TravisBuddy Request Identifier: 327c6100-095e-11eb-b9d6-41d4da7e5060

@poyea poyea changed the title test/graphs/kruskal: adding a test case to verify the correctness of the algorithm graphs/kruskal: add a test case to verify the correctness, fix styles Oct 8, 2020
poyea
poyea approved these changes Oct 8, 2020
View reviewed changes

@poyea poyea left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your pull request!🤩

@poyea poyea added the awaiting merge This PR is approved and ready to be merged label Oct 8, 2020
@poyea
poyea merged commit c2a5033 into TheAlgorithms:master Oct 8, 2020
@cclauss

cclauss commented Oct 8, 2020

Copy link
Copy Markdown
Member

No type hints or doctests.

@poyea

poyea commented Oct 8, 2020

Copy link
Copy Markdown
Member

Right we can add some type hints to this file @meysam81

@meysam81
meysam81 deleted the test/graphs/kruskal branch October 9, 2020 07:55
@meysam81 meysam81 mentioned this pull request Oct 9, 2020
Merged
11 of 14 tasks
cclauss pushed a commit that referenced this pull request Oct 15, 2020
@meysam81 github-actions
Graphs/kruskal: adding doctest & type hints (#3101)
83b8250 
* graphs/kruskal: add doctest & type hints

this is a child of a previous PR #2443

its ancestor is #2128

* updating DIRECTORY.md

* graphs/kruskal: fix max-line-length violation

* fixup! Format Python code with psf/black push

Co-authored-by: github-actions <${GITHUB_ACTOR}@users.noreply.github.com>
cclauss
cclauss reviewed Oct 16, 2020
View reviewed changes
Comment thread graphs/minimum_spanning_tree_kruskal.py Outdated
edges.append((i, node1, node2, cost))

edges = sorted(edges, key=lambda edge: edge[3])
def run_algorithm(num_nodes, num_edges, edges):

@cclauss cclauss Sep 18, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add Python type hints on function parameters and the return type.

Does num_edges always equal len(edges)? If so, let’s not force the caller to provide us num_edges.

@meysam81 meysam81 Oct 16, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding type hints was done in #3101 which you merged a couple of hours ago.
We can remove num_edges if you say so.

@cclauss cclauss Oct 16, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These comments were on my second computer and I never hit Send... Thanks for doing this.

@meysam81 meysam81 Oct 16, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My pleasure 🥂

stokhos pushed a commit to stokhos/Python that referenced this pull request Jan 3, 2021
@meysam81 @cclauss
@poyea github-actions @stokhos
graphs/kruskal: add a test case to verify the correctness, fix styles (…
4b2e30a 
…TheAlgorithms#2443)

* test/graphs/kruskal: adding a test case to verify the correctness of the algorithm

Fixes TheAlgorithms#2128

* grahps/kruskal: running psf/black

* graphs/kruskal: read edges in a friendlier fashion

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

* Update minimum_spanning_tree_kruskal.py

* fixup! Format Python code with psf/black push

* Update test_min_spanning_tree_kruskal.py

* updating DIRECTORY.md

Co-authored-by: Christian Clauss <cclauss@me.com>
Co-authored-by: John Law <johnlaw.po@gmail.com>
Co-authored-by: github-actions <${GITHUB_ACTOR}@users.noreply.github.com>
stokhos pushed a commit to stokhos/Python that referenced this pull request Jan 3, 2021
@meysam81 github-actions @stokhos
Graphs/kruskal: adding doctest & type hints (TheAlgorithms#3101)
b6ccf95 
* graphs/kruskal: add doctest & type hints

this is a child of a previous PR TheAlgorithms#2443

its ancestor is TheAlgorithms#2128

* updating DIRECTORY.md

* graphs/kruskal: fix max-line-length violation

* fixup! Format Python code with psf/black push

Co-authored-by: github-actions <${GITHUB_ACTOR}@users.noreply.github.com>
Panquesito7 pushed a commit to Panquesito7/Python that referenced this pull request May 13, 2021
@meysam81 @cclauss
@poyea github-actions
graphs/kruskal: add a test case to verify the correctness, fix styles (…
d493b81 
…TheAlgorithms#2443)

* test/graphs/kruskal: adding a test case to verify the correctness of the algorithm

Fixes TheAlgorithms#2128

* grahps/kruskal: running psf/black

* graphs/kruskal: read edges in a friendlier fashion

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

* Update minimum_spanning_tree_kruskal.py

* fixup! Format Python code with psf/black push

* Update test_min_spanning_tree_kruskal.py

* updating DIRECTORY.md

Co-authored-by: Christian Clauss <cclauss@me.com>
Co-authored-by: John Law <johnlaw.po@gmail.com>
Co-authored-by: github-actions <${GITHUB_ACTOR}@users.noreply.github.com>
Panquesito7 pushed a commit to Panquesito7/Python that referenced this pull request May 13, 2021
@meysam81 github-actions
Graphs/kruskal: adding doctest & type hints (TheAlgorithms#3101)
c3e96d2 
* graphs/kruskal: add doctest & type hints

this is a child of a previous PR TheAlgorithms#2443

its ancestor is TheAlgorithms#2128

* updating DIRECTORY.md

* graphs/kruskal: fix max-line-length violation

* fixup! Format Python code with psf/black push

Co-authored-by: github-actions <${GITHUB_ACTOR}@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cclauss cclauss cclauss left review comments
@poyea poyea poyea approved these changes

Assignees

@poyea poyea

Labels

awaiting merge This PR is approved and ready to be merged

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@meysam81 @TravisBuddy @cclauss @poyea