Skip to content

Improved Graph Implementations - #8730

Merged
cclauss merged 12 commits into
TheAlgorithms:masterfrom
nith2001:master
May 31, 2023
Merged

Improved Graph Implementations#8730
cclauss merged 12 commits into
TheAlgorithms:masterfrom
nith2001:master

Conversation

@nith2001

@nith2001nith2001 commented May 14, 2023

Copy link
Copy Markdown
Contributor

Describe your change:

The graph implementations using the adjacency list and adjacency matrix were not very comprehensive and lacked supporting functions. I wanted to improve them and also write tests to prove they worked. This also solved Issue #8709, which I brought up. To run my tests, do python3 <testfile>.py under the graphs/tests folder.

  • 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.
    ^^ it changes two, but they're both about the same thing, which is graph implementation. Let me know if that's not okay.
  • 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 include at least one URL that points to Wikipedia or another similar explanation.
  • If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.

Provides new implementation for graph_list.py and graph_matrix.py along with pytest suites for each. FixesTheAlgorithms#8709
@nith2001

nith2001 commented May 14, 2023

Copy link
Copy Markdown
ContributorAuthor

I'll clear up the failed code quality test soon. First-time contributor so I didn't read the Contributor.md as closely as I should've regarding testing and quality.

@algorithms-keeperalgorithms-keeperBot added the tests are failing Do not merge until tests pass label May 14, 2023
Comment threadgraphs/graph_list.py Outdated
@nith2001

nith2001 commented May 15, 2023

Copy link
Copy Markdown
ContributorAuthor

I wrote a bunch of unit tests using the unittest framework rather than doctest because I didn't want to make my code super unnecessarily long. Is that okay?

@algorithms-keeperalgorithms-keeperBot removed the tests are failing Do not merge until tests pass label May 15, 2023
@nith2001

Copy link
Copy Markdown
ContributorAuthor

Not yet, gonna push some comments and docs describing the implementation

@algorithms-keeperalgorithms-keeperBot added the awaiting reviews This PR is ready to be reviewed label May 15, 2023
@nith2001

nith2001 commented May 15, 2023

Copy link
Copy Markdown
ContributorAuthor

Alright, all done. I wanted to store my tests in the graphs/tests folder but I couldn't find a way to import my graph classes without violated some type of black linter code quality. I wanted to do sys.path.append("..") and then import the class but black and ruff wouldn't let me I think. Otherwise, let me know what I can fix!

@nith2001
nith2001 requested a review from cclaussMay 15, 2023 20:59
@nith2001

Copy link
Copy Markdown
ContributorAuthor

Are y'all backlogged with work? Just checking in.

Comment threadgraphs/graph_list.py
Comment threadgraphs/graph_list.py Outdated
Comment threadgraphs/graph_adj_list.py
Comment threadgraphs/graph_adj_list.py Outdated
Comment threadgraphs/graph_adj_list.py Outdated
Comment threadgraphs/graph_adj_list.py Outdated
Comment threadgraphs/graph_adj_list.py Outdated
@nith2001nith2001 changed the title Improved Graph Implementations #8709Improved Graph ImplementationsMay 25, 2023
@nith2001

Copy link
Copy Markdown
ContributorAuthor

Ready for another review!

Comment threadgraphs/graph_adjacency_list.py
@algorithms-keeperalgorithms-keeperBot added the require type hints https://docs.python.org/3/library/typing.html label May 30, 2023

@algorithms-keeperalgorithms-keeperBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Click here to look at the relevant links ⬇️

🔗 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 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.

Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
@algorithms-keeperalgorithms-keeperBot added the tests are failing Do not merge until tests pass label May 30, 2023

@algorithms-keeperalgorithms-keeperBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Click here to look at the relevant links ⬇️

🔗 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 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.

Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
@algorithms-keeperalgorithms-keeperBot removed the tests are failing Do not merge until tests pass label May 30, 2023

@algorithms-keeperalgorithms-keeperBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Click here to look at the relevant links ⬇️

🔗 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 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.

Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py
Comment threadgraphs/graph_adjacency_matrix.py
Comment threadgraphs/graph_adjacency_matrix.py
Comment threadgraphs/graph_adjacency_matrix.py

@algorithms-keeperalgorithms-keeperBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Click here to look at the relevant links ⬇️

🔗 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 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.

Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
@algorithms-keeperalgorithms-keeperBot removed the require type hints https://docs.python.org/3/library/typing.html label May 30, 2023
@nith2001
nith2001 requested a review from cclaussMay 30, 2023 21:53
@nith2001

nith2001 commented May 30, 2023

Copy link
Copy Markdown
ContributorAuthor

Took a few too many commits to resolve the type hints issue and a new ruff issue that popped up about f-strings in exceptions that didn't show up on the local ruff check. As always, appreciate the feedback!

Edit: Still ready for review, the commit below was just a minor fix I needed to make in my error messaging grammar.

@algorithms-keeperalgorithms-keeperBot removed the awaiting reviews This PR is ready to be reviewed label May 31, 2023
@cclauss
cclauss merged commit 4621b0b into TheAlgorithms:masterMay 31, 2023
@nith2001

Copy link
Copy Markdown
ContributorAuthor

Thanks!

@isidroasisidroas mentioned this pull request Jan 25, 2025
14 tasks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@nith2001@cclauss
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Improved Graph Implementations by nith2001 · Pull Request #8730 · TheAlgorithms/Python · GitHub
Skip to content

Improved Graph Implementations - #8730

Merged
cclauss merged 12 commits into
TheAlgorithms:masterfrom
nith2001:master
May 31, 2023
Merged

Improved Graph Implementations#8730
cclauss merged 12 commits into
TheAlgorithms:masterfrom
nith2001:master

Conversation

@nith2001

@nith2001nith2001 commented May 14, 2023

Copy link
Copy Markdown
Contributor

Describe your change:

The graph implementations using the adjacency list and adjacency matrix were not very comprehensive and lacked supporting functions. I wanted to improve them and also write tests to prove they worked. This also solved Issue #8709, which I brought up. To run my tests, do python3 <testfile>.py under the graphs/tests folder.

  • 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.
    ^^ it changes two, but they're both about the same thing, which is graph implementation. Let me know if that's not okay.
  • 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 include at least one URL that points to Wikipedia or another similar explanation.
  • If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.

Provides new implementation for graph_list.py and graph_matrix.py along with pytest suites for each. FixesTheAlgorithms#8709
@nith2001

nith2001 commented May 14, 2023

Copy link
Copy Markdown
ContributorAuthor

I'll clear up the failed code quality test soon. First-time contributor so I didn't read the Contributor.md as closely as I should've regarding testing and quality.

@algorithms-keeperalgorithms-keeperBot added the tests are failing Do not merge until tests pass label May 14, 2023
Comment threadgraphs/graph_list.py Outdated
@nith2001

nith2001 commented May 15, 2023

Copy link
Copy Markdown
ContributorAuthor

I wrote a bunch of unit tests using the unittest framework rather than doctest because I didn't want to make my code super unnecessarily long. Is that okay?

@algorithms-keeperalgorithms-keeperBot removed the tests are failing Do not merge until tests pass label May 15, 2023
@nith2001

Copy link
Copy Markdown
ContributorAuthor

Not yet, gonna push some comments and docs describing the implementation

@algorithms-keeperalgorithms-keeperBot added the awaiting reviews This PR is ready to be reviewed label May 15, 2023
@nith2001

nith2001 commented May 15, 2023

Copy link
Copy Markdown
ContributorAuthor

Alright, all done. I wanted to store my tests in the graphs/tests folder but I couldn't find a way to import my graph classes without violated some type of black linter code quality. I wanted to do sys.path.append("..") and then import the class but black and ruff wouldn't let me I think. Otherwise, let me know what I can fix!

@nith2001
nith2001 requested a review from cclaussMay 15, 2023 20:59
@nith2001

Copy link
Copy Markdown
ContributorAuthor

Are y'all backlogged with work? Just checking in.

Comment threadgraphs/graph_list.py
Comment threadgraphs/graph_list.py Outdated
Comment threadgraphs/graph_adj_list.py
Comment threadgraphs/graph_adj_list.py Outdated
Comment threadgraphs/graph_adj_list.py Outdated
Comment threadgraphs/graph_adj_list.py Outdated
Comment threadgraphs/graph_adj_list.py Outdated
@nith2001nith2001 changed the title Improved Graph Implementations #8709Improved Graph ImplementationsMay 25, 2023
@nith2001

Copy link
Copy Markdown
ContributorAuthor

Ready for another review!

Comment threadgraphs/graph_adjacency_list.py
@algorithms-keeperalgorithms-keeperBot added the require type hints https://docs.python.org/3/library/typing.html label May 30, 2023

@algorithms-keeperalgorithms-keeperBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Click here to look at the relevant links ⬇️

🔗 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 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.

Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
@algorithms-keeperalgorithms-keeperBot added the tests are failing Do not merge until tests pass label May 30, 2023

@algorithms-keeperalgorithms-keeperBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Click here to look at the relevant links ⬇️

🔗 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 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.

Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
@algorithms-keeperalgorithms-keeperBot removed the tests are failing Do not merge until tests pass label May 30, 2023

@algorithms-keeperalgorithms-keeperBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Click here to look at the relevant links ⬇️

🔗 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 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.

Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py
Comment threadgraphs/graph_adjacency_matrix.py
Comment threadgraphs/graph_adjacency_matrix.py
Comment threadgraphs/graph_adjacency_matrix.py

@algorithms-keeperalgorithms-keeperBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Click here to look at the relevant links ⬇️

🔗 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 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.

Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
@algorithms-keeperalgorithms-keeperBot removed the require type hints https://docs.python.org/3/library/typing.html label May 30, 2023
@nith2001
nith2001 requested a review from cclaussMay 30, 2023 21:53
@nith2001

nith2001 commented May 30, 2023

Copy link
Copy Markdown
ContributorAuthor

Took a few too many commits to resolve the type hints issue and a new ruff issue that popped up about f-strings in exceptions that didn't show up on the local ruff check. As always, appreciate the feedback!

Edit: Still ready for review, the commit below was just a minor fix I needed to make in my error messaging grammar.

@algorithms-keeperalgorithms-keeperBot removed the awaiting reviews This PR is ready to be reviewed label May 31, 2023
@cclauss
cclauss merged commit 4621b0b into TheAlgorithms:masterMay 31, 2023
@nith2001

Copy link
Copy Markdown
ContributorAuthor

Thanks!

@isidroasisidroas mentioned this pull request Jan 25, 2025
14 tasks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@nith2001@cclauss
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Improved Graph Implementations by nith2001 · Pull Request #8730 · TheAlgorithms/Python · GitHub
Skip to content

Improved Graph Implementations - #8730

Merged
cclauss merged 12 commits into
TheAlgorithms:masterfrom
nith2001:master
May 31, 2023
Merged

Improved Graph Implementations#8730
cclauss merged 12 commits into
TheAlgorithms:masterfrom
nith2001:master

Conversation

@nith2001

@nith2001nith2001 commented May 14, 2023

Copy link
Copy Markdown
Contributor

Describe your change:

The graph implementations using the adjacency list and adjacency matrix were not very comprehensive and lacked supporting functions. I wanted to improve them and also write tests to prove they worked. This also solved Issue #8709, which I brought up. To run my tests, do python3 <testfile>.py under the graphs/tests folder.

  • 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.
    ^^ it changes two, but they're both about the same thing, which is graph implementation. Let me know if that's not okay.
  • 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 include at least one URL that points to Wikipedia or another similar explanation.
  • If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.

Provides new implementation for graph_list.py and graph_matrix.py along with pytest suites for each. FixesTheAlgorithms#8709
@nith2001

nith2001 commented May 14, 2023

Copy link
Copy Markdown
ContributorAuthor

I'll clear up the failed code quality test soon. First-time contributor so I didn't read the Contributor.md as closely as I should've regarding testing and quality.

@algorithms-keeperalgorithms-keeperBot added the tests are failing Do not merge until tests pass label May 14, 2023
Comment threadgraphs/graph_list.py Outdated
@nith2001

nith2001 commented May 15, 2023

Copy link
Copy Markdown
ContributorAuthor

I wrote a bunch of unit tests using the unittest framework rather than doctest because I didn't want to make my code super unnecessarily long. Is that okay?

@algorithms-keeperalgorithms-keeperBot removed the tests are failing Do not merge until tests pass label May 15, 2023
@nith2001

Copy link
Copy Markdown
ContributorAuthor

Not yet, gonna push some comments and docs describing the implementation

@algorithms-keeperalgorithms-keeperBot added the awaiting reviews This PR is ready to be reviewed label May 15, 2023
@nith2001

nith2001 commented May 15, 2023

Copy link
Copy Markdown
ContributorAuthor

Alright, all done. I wanted to store my tests in the graphs/tests folder but I couldn't find a way to import my graph classes without violated some type of black linter code quality. I wanted to do sys.path.append("..") and then import the class but black and ruff wouldn't let me I think. Otherwise, let me know what I can fix!

@nith2001
nith2001 requested a review from cclaussMay 15, 2023 20:59
@nith2001

Copy link
Copy Markdown
ContributorAuthor

Are y'all backlogged with work? Just checking in.

Comment threadgraphs/graph_list.py
Comment threadgraphs/graph_list.py Outdated
Comment threadgraphs/graph_adj_list.py
Comment threadgraphs/graph_adj_list.py Outdated
Comment threadgraphs/graph_adj_list.py Outdated
Comment threadgraphs/graph_adj_list.py Outdated
Comment threadgraphs/graph_adj_list.py Outdated
@nith2001nith2001 changed the title Improved Graph Implementations #8709Improved Graph ImplementationsMay 25, 2023
@nith2001

Copy link
Copy Markdown
ContributorAuthor

Ready for another review!

Comment threadgraphs/graph_adjacency_list.py
@algorithms-keeperalgorithms-keeperBot added the require type hints https://docs.python.org/3/library/typing.html label May 30, 2023

@algorithms-keeperalgorithms-keeperBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Click here to look at the relevant links ⬇️

🔗 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 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.

Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
@algorithms-keeperalgorithms-keeperBot added the tests are failing Do not merge until tests pass label May 30, 2023

@algorithms-keeperalgorithms-keeperBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Click here to look at the relevant links ⬇️

🔗 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 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.

Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
@algorithms-keeperalgorithms-keeperBot removed the tests are failing Do not merge until tests pass label May 30, 2023

@algorithms-keeperalgorithms-keeperBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Click here to look at the relevant links ⬇️

🔗 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 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.

Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py
Comment threadgraphs/graph_adjacency_matrix.py
Comment threadgraphs/graph_adjacency_matrix.py
Comment threadgraphs/graph_adjacency_matrix.py

@algorithms-keeperalgorithms-keeperBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Click here to look at the relevant links ⬇️

🔗 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 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.

Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
@algorithms-keeperalgorithms-keeperBot removed the require type hints https://docs.python.org/3/library/typing.html label May 30, 2023
@nith2001
nith2001 requested a review from cclaussMay 30, 2023 21:53
@nith2001

nith2001 commented May 30, 2023

Copy link
Copy Markdown
ContributorAuthor

Took a few too many commits to resolve the type hints issue and a new ruff issue that popped up about f-strings in exceptions that didn't show up on the local ruff check. As always, appreciate the feedback!

Edit: Still ready for review, the commit below was just a minor fix I needed to make in my error messaging grammar.

@algorithms-keeperalgorithms-keeperBot removed the awaiting reviews This PR is ready to be reviewed label May 31, 2023
@cclauss
cclauss merged commit 4621b0b into TheAlgorithms:masterMay 31, 2023
@nith2001

Copy link
Copy Markdown
ContributorAuthor

Thanks!

@isidroasisidroas mentioned this pull request Jan 25, 2025
14 tasks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@nith2001@cclauss
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Improved Graph Implementations by nith2001 · Pull Request #8730 · TheAlgorithms/Python · GitHub
Skip to content

Improved Graph Implementations - #8730

Merged
cclauss merged 12 commits into
TheAlgorithms:masterfrom
nith2001:master
May 31, 2023
Merged

Improved Graph Implementations#8730
cclauss merged 12 commits into
TheAlgorithms:masterfrom
nith2001:master

Conversation

@nith2001

@nith2001nith2001 commented May 14, 2023

Copy link
Copy Markdown
Contributor

Describe your change:

The graph implementations using the adjacency list and adjacency matrix were not very comprehensive and lacked supporting functions. I wanted to improve them and also write tests to prove they worked. This also solved Issue #8709, which I brought up. To run my tests, do python3 <testfile>.py under the graphs/tests folder.

  • 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.
    ^^ it changes two, but they're both about the same thing, which is graph implementation. Let me know if that's not okay.
  • 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 include at least one URL that points to Wikipedia or another similar explanation.
  • If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.

Provides new implementation for graph_list.py and graph_matrix.py along with pytest suites for each. FixesTheAlgorithms#8709
@nith2001

nith2001 commented May 14, 2023

Copy link
Copy Markdown
ContributorAuthor

I'll clear up the failed code quality test soon. First-time contributor so I didn't read the Contributor.md as closely as I should've regarding testing and quality.

@algorithms-keeperalgorithms-keeperBot added the tests are failing Do not merge until tests pass label May 14, 2023
Comment threadgraphs/graph_list.py Outdated
@nith2001

nith2001 commented May 15, 2023

Copy link
Copy Markdown
ContributorAuthor

I wrote a bunch of unit tests using the unittest framework rather than doctest because I didn't want to make my code super unnecessarily long. Is that okay?

@algorithms-keeperalgorithms-keeperBot removed the tests are failing Do not merge until tests pass label May 15, 2023
@nith2001

Copy link
Copy Markdown
ContributorAuthor

Not yet, gonna push some comments and docs describing the implementation

@algorithms-keeperalgorithms-keeperBot added the awaiting reviews This PR is ready to be reviewed label May 15, 2023
@nith2001

nith2001 commented May 15, 2023

Copy link
Copy Markdown
ContributorAuthor

Alright, all done. I wanted to store my tests in the graphs/tests folder but I couldn't find a way to import my graph classes without violated some type of black linter code quality. I wanted to do sys.path.append("..") and then import the class but black and ruff wouldn't let me I think. Otherwise, let me know what I can fix!

@nith2001
nith2001 requested a review from cclaussMay 15, 2023 20:59
@nith2001

Copy link
Copy Markdown
ContributorAuthor

Are y'all backlogged with work? Just checking in.

Comment threadgraphs/graph_list.py
Comment threadgraphs/graph_list.py Outdated
Comment threadgraphs/graph_adj_list.py
Comment threadgraphs/graph_adj_list.py Outdated
Comment threadgraphs/graph_adj_list.py Outdated
Comment threadgraphs/graph_adj_list.py Outdated
Comment threadgraphs/graph_adj_list.py Outdated
@nith2001nith2001 changed the title Improved Graph Implementations #8709Improved Graph ImplementationsMay 25, 2023
@nith2001

Copy link
Copy Markdown
ContributorAuthor

Ready for another review!

Comment threadgraphs/graph_adjacency_list.py
@algorithms-keeperalgorithms-keeperBot added the require type hints https://docs.python.org/3/library/typing.html label May 30, 2023

@algorithms-keeperalgorithms-keeperBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Click here to look at the relevant links ⬇️

🔗 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 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.

Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
@algorithms-keeperalgorithms-keeperBot added the tests are failing Do not merge until tests pass label May 30, 2023

@algorithms-keeperalgorithms-keeperBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Click here to look at the relevant links ⬇️

🔗 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 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.

Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
@algorithms-keeperalgorithms-keeperBot removed the tests are failing Do not merge until tests pass label May 30, 2023

@algorithms-keeperalgorithms-keeperBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Click here to look at the relevant links ⬇️

🔗 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 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.

Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py
Comment threadgraphs/graph_adjacency_matrix.py
Comment threadgraphs/graph_adjacency_matrix.py
Comment threadgraphs/graph_adjacency_matrix.py

@algorithms-keeperalgorithms-keeperBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Click here to look at the relevant links ⬇️

🔗 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 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.

Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
@algorithms-keeperalgorithms-keeperBot removed the require type hints https://docs.python.org/3/library/typing.html label May 30, 2023
@nith2001
nith2001 requested a review from cclaussMay 30, 2023 21:53
@nith2001

nith2001 commented May 30, 2023

Copy link
Copy Markdown
ContributorAuthor

Took a few too many commits to resolve the type hints issue and a new ruff issue that popped up about f-strings in exceptions that didn't show up on the local ruff check. As always, appreciate the feedback!

Edit: Still ready for review, the commit below was just a minor fix I needed to make in my error messaging grammar.

@algorithms-keeperalgorithms-keeperBot removed the awaiting reviews This PR is ready to be reviewed label May 31, 2023
@cclauss
cclauss merged commit 4621b0b into TheAlgorithms:masterMay 31, 2023
@nith2001

Copy link
Copy Markdown
ContributorAuthor

Thanks!

@isidroasisidroas mentioned this pull request Jan 25, 2025
14 tasks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@nith2001@cclauss
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' Improved Graph Implementations by nith2001 · Pull Request #8730 · TheAlgorithms/Python · GitHub
Skip to content

Improved Graph Implementations - #8730

Merged
cclauss merged 12 commits into
TheAlgorithms:masterfrom
nith2001:master
May 31, 2023
Merged

Improved Graph Implementations#8730
cclauss merged 12 commits into
TheAlgorithms:masterfrom
nith2001:master

Conversation

@nith2001

@nith2001nith2001 commented May 14, 2023

Copy link
Copy Markdown
Contributor

Describe your change:

The graph implementations using the adjacency list and adjacency matrix were not very comprehensive and lacked supporting functions. I wanted to improve them and also write tests to prove they worked. This also solved Issue #8709, which I brought up. To run my tests, do python3 <testfile>.py under the graphs/tests folder.

  • 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.
    ^^ it changes two, but they're both about the same thing, which is graph implementation. Let me know if that's not okay.
  • 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 include at least one URL that points to Wikipedia or another similar explanation.
  • If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.

Provides new implementation for graph_list.py and graph_matrix.py along with pytest suites for each. FixesTheAlgorithms#8709
@nith2001

nith2001 commented May 14, 2023

Copy link
Copy Markdown
ContributorAuthor

I'll clear up the failed code quality test soon. First-time contributor so I didn't read the Contributor.md as closely as I should've regarding testing and quality.

@algorithms-keeperalgorithms-keeperBot added the tests are failing Do not merge until tests pass label May 14, 2023
Comment threadgraphs/graph_list.py Outdated
@nith2001

nith2001 commented May 15, 2023

Copy link
Copy Markdown
ContributorAuthor

I wrote a bunch of unit tests using the unittest framework rather than doctest because I didn't want to make my code super unnecessarily long. Is that okay?

@algorithms-keeperalgorithms-keeperBot removed the tests are failing Do not merge until tests pass label May 15, 2023
@nith2001

Copy link
Copy Markdown
ContributorAuthor

Not yet, gonna push some comments and docs describing the implementation

@algorithms-keeperalgorithms-keeperBot added the awaiting reviews This PR is ready to be reviewed label May 15, 2023
@nith2001

nith2001 commented May 15, 2023

Copy link
Copy Markdown
ContributorAuthor

Alright, all done. I wanted to store my tests in the graphs/tests folder but I couldn't find a way to import my graph classes without violated some type of black linter code quality. I wanted to do sys.path.append("..") and then import the class but black and ruff wouldn't let me I think. Otherwise, let me know what I can fix!

@nith2001
nith2001 requested a review from cclaussMay 15, 2023 20:59
@nith2001

Copy link
Copy Markdown
ContributorAuthor

Are y'all backlogged with work? Just checking in.

Comment threadgraphs/graph_list.py
Comment threadgraphs/graph_list.py Outdated
Comment threadgraphs/graph_adj_list.py
Comment threadgraphs/graph_adj_list.py Outdated
Comment threadgraphs/graph_adj_list.py Outdated
Comment threadgraphs/graph_adj_list.py Outdated
Comment threadgraphs/graph_adj_list.py Outdated
@nith2001nith2001 changed the title Improved Graph Implementations #8709Improved Graph ImplementationsMay 25, 2023
@nith2001

Copy link
Copy Markdown
ContributorAuthor

Ready for another review!

Comment threadgraphs/graph_adjacency_list.py
@algorithms-keeperalgorithms-keeperBot added the require type hints https://docs.python.org/3/library/typing.html label May 30, 2023

@algorithms-keeperalgorithms-keeperBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Click here to look at the relevant links ⬇️

🔗 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 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.

Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
@algorithms-keeperalgorithms-keeperBot added the tests are failing Do not merge until tests pass label May 30, 2023

@algorithms-keeperalgorithms-keeperBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Click here to look at the relevant links ⬇️

🔗 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 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.

Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
@algorithms-keeperalgorithms-keeperBot removed the tests are failing Do not merge until tests pass label May 30, 2023

@algorithms-keeperalgorithms-keeperBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Click here to look at the relevant links ⬇️

🔗 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 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.

Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py
Comment threadgraphs/graph_adjacency_matrix.py
Comment threadgraphs/graph_adjacency_matrix.py
Comment threadgraphs/graph_adjacency_matrix.py

@algorithms-keeperalgorithms-keeperBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Click here to look at the relevant links ⬇️

🔗 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 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.

Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
@algorithms-keeperalgorithms-keeperBot removed the require type hints https://docs.python.org/3/library/typing.html label May 30, 2023
@nith2001
nith2001 requested a review from cclaussMay 30, 2023 21:53
@nith2001

nith2001 commented May 30, 2023

Copy link
Copy Markdown
ContributorAuthor

Took a few too many commits to resolve the type hints issue and a new ruff issue that popped up about f-strings in exceptions that didn't show up on the local ruff check. As always, appreciate the feedback!

Edit: Still ready for review, the commit below was just a minor fix I needed to make in my error messaging grammar.

@algorithms-keeperalgorithms-keeperBot removed the awaiting reviews This PR is ready to be reviewed label May 31, 2023
@cclauss
cclauss merged commit 4621b0b into TheAlgorithms:masterMay 31, 2023
@nith2001

Copy link
Copy Markdown
ContributorAuthor

Thanks!

@isidroasisidroas mentioned this pull request Jan 25, 2025
14 tasks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@nith2001@cclauss
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Improved Graph Implementations by nith2001 · Pull Request #8730 · TheAlgorithms/Python · GitHub
Skip to content

Improved Graph Implementations - #8730

Merged
cclauss merged 12 commits into
TheAlgorithms:masterfrom
nith2001:master
May 31, 2023
Merged

Improved Graph Implementations#8730
cclauss merged 12 commits into
TheAlgorithms:masterfrom
nith2001:master

Conversation

@nith2001

@nith2001nith2001 commented May 14, 2023

Copy link
Copy Markdown
Contributor

Describe your change:

The graph implementations using the adjacency list and adjacency matrix were not very comprehensive and lacked supporting functions. I wanted to improve them and also write tests to prove they worked. This also solved Issue #8709, which I brought up. To run my tests, do python3 <testfile>.py under the graphs/tests folder.

  • 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.
    ^^ it changes two, but they're both about the same thing, which is graph implementation. Let me know if that's not okay.
  • 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 include at least one URL that points to Wikipedia or another similar explanation.
  • If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.

Provides new implementation for graph_list.py and graph_matrix.py along with pytest suites for each. FixesTheAlgorithms#8709
@nith2001

nith2001 commented May 14, 2023

Copy link
Copy Markdown
ContributorAuthor

I'll clear up the failed code quality test soon. First-time contributor so I didn't read the Contributor.md as closely as I should've regarding testing and quality.

@algorithms-keeperalgorithms-keeperBot added the tests are failing Do not merge until tests pass label May 14, 2023
Comment threadgraphs/graph_list.py Outdated
@nith2001

nith2001 commented May 15, 2023

Copy link
Copy Markdown
ContributorAuthor

I wrote a bunch of unit tests using the unittest framework rather than doctest because I didn't want to make my code super unnecessarily long. Is that okay?

@algorithms-keeperalgorithms-keeperBot removed the tests are failing Do not merge until tests pass label May 15, 2023
@nith2001

Copy link
Copy Markdown
ContributorAuthor

Not yet, gonna push some comments and docs describing the implementation

@algorithms-keeperalgorithms-keeperBot added the awaiting reviews This PR is ready to be reviewed label May 15, 2023
@nith2001

nith2001 commented May 15, 2023

Copy link
Copy Markdown
ContributorAuthor

Alright, all done. I wanted to store my tests in the graphs/tests folder but I couldn't find a way to import my graph classes without violated some type of black linter code quality. I wanted to do sys.path.append("..") and then import the class but black and ruff wouldn't let me I think. Otherwise, let me know what I can fix!

@nith2001
nith2001 requested a review from cclaussMay 15, 2023 20:59
@nith2001

Copy link
Copy Markdown
ContributorAuthor

Are y'all backlogged with work? Just checking in.

Comment threadgraphs/graph_list.py
Comment threadgraphs/graph_list.py Outdated
Comment threadgraphs/graph_adj_list.py
Comment threadgraphs/graph_adj_list.py Outdated
Comment threadgraphs/graph_adj_list.py Outdated
Comment threadgraphs/graph_adj_list.py Outdated
Comment threadgraphs/graph_adj_list.py Outdated
@nith2001nith2001 changed the title Improved Graph Implementations #8709Improved Graph ImplementationsMay 25, 2023
@nith2001

Copy link
Copy Markdown
ContributorAuthor

Ready for another review!

Comment threadgraphs/graph_adjacency_list.py
@algorithms-keeperalgorithms-keeperBot added the require type hints https://docs.python.org/3/library/typing.html label May 30, 2023

@algorithms-keeperalgorithms-keeperBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Click here to look at the relevant links ⬇️

🔗 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 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.

Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
@algorithms-keeperalgorithms-keeperBot added the tests are failing Do not merge until tests pass label May 30, 2023

@algorithms-keeperalgorithms-keeperBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Click here to look at the relevant links ⬇️

🔗 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 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.

Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
@algorithms-keeperalgorithms-keeperBot removed the tests are failing Do not merge until tests pass label May 30, 2023

@algorithms-keeperalgorithms-keeperBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Click here to look at the relevant links ⬇️

🔗 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 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.

Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py
Comment threadgraphs/graph_adjacency_matrix.py
Comment threadgraphs/graph_adjacency_matrix.py
Comment threadgraphs/graph_adjacency_matrix.py

@algorithms-keeperalgorithms-keeperBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Click here to look at the relevant links ⬇️

🔗 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 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.

Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
@algorithms-keeperalgorithms-keeperBot removed the require type hints https://docs.python.org/3/library/typing.html label May 30, 2023
@nith2001
nith2001 requested a review from cclaussMay 30, 2023 21:53
@nith2001

nith2001 commented May 30, 2023

Copy link
Copy Markdown
ContributorAuthor

Took a few too many commits to resolve the type hints issue and a new ruff issue that popped up about f-strings in exceptions that didn't show up on the local ruff check. As always, appreciate the feedback!

Edit: Still ready for review, the commit below was just a minor fix I needed to make in my error messaging grammar.

@algorithms-keeperalgorithms-keeperBot removed the awaiting reviews This PR is ready to be reviewed label May 31, 2023
@cclauss
cclauss merged commit 4621b0b into TheAlgorithms:masterMay 31, 2023
@nith2001

Copy link
Copy Markdown
ContributorAuthor

Thanks!

@isidroasisidroas mentioned this pull request Jan 25, 2025
14 tasks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@nith2001@cclauss
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Improved Graph Implementations by nith2001 · Pull Request #8730 · TheAlgorithms/Python · GitHub
Skip to content

Improved Graph Implementations - #8730

Merged
cclauss merged 12 commits into
TheAlgorithms:masterfrom
nith2001:master
May 31, 2023
Merged

Improved Graph Implementations#8730
cclauss merged 12 commits into
TheAlgorithms:masterfrom
nith2001:master

Conversation

@nith2001

@nith2001nith2001 commented May 14, 2023

Copy link
Copy Markdown
Contributor

Describe your change:

The graph implementations using the adjacency list and adjacency matrix were not very comprehensive and lacked supporting functions. I wanted to improve them and also write tests to prove they worked. This also solved Issue #8709, which I brought up. To run my tests, do python3 <testfile>.py under the graphs/tests folder.

  • 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.
    ^^ it changes two, but they're both about the same thing, which is graph implementation. Let me know if that's not okay.
  • 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 include at least one URL that points to Wikipedia or another similar explanation.
  • If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.

Provides new implementation for graph_list.py and graph_matrix.py along with pytest suites for each. FixesTheAlgorithms#8709
@nith2001

nith2001 commented May 14, 2023

Copy link
Copy Markdown
ContributorAuthor

I'll clear up the failed code quality test soon. First-time contributor so I didn't read the Contributor.md as closely as I should've regarding testing and quality.

@algorithms-keeperalgorithms-keeperBot added the tests are failing Do not merge until tests pass label May 14, 2023
Comment threadgraphs/graph_list.py Outdated
@nith2001

nith2001 commented May 15, 2023

Copy link
Copy Markdown
ContributorAuthor

I wrote a bunch of unit tests using the unittest framework rather than doctest because I didn't want to make my code super unnecessarily long. Is that okay?

@algorithms-keeperalgorithms-keeperBot removed the tests are failing Do not merge until tests pass label May 15, 2023
@nith2001

Copy link
Copy Markdown
ContributorAuthor

Not yet, gonna push some comments and docs describing the implementation

@algorithms-keeperalgorithms-keeperBot added the awaiting reviews This PR is ready to be reviewed label May 15, 2023
@nith2001

nith2001 commented May 15, 2023

Copy link
Copy Markdown
ContributorAuthor

Alright, all done. I wanted to store my tests in the graphs/tests folder but I couldn't find a way to import my graph classes without violated some type of black linter code quality. I wanted to do sys.path.append("..") and then import the class but black and ruff wouldn't let me I think. Otherwise, let me know what I can fix!

@nith2001
nith2001 requested a review from cclaussMay 15, 2023 20:59
@nith2001

Copy link
Copy Markdown
ContributorAuthor

Are y'all backlogged with work? Just checking in.

Comment threadgraphs/graph_list.py
Comment threadgraphs/graph_list.py Outdated
Comment threadgraphs/graph_adj_list.py
Comment threadgraphs/graph_adj_list.py Outdated
Comment threadgraphs/graph_adj_list.py Outdated
Comment threadgraphs/graph_adj_list.py Outdated
Comment threadgraphs/graph_adj_list.py Outdated
@nith2001nith2001 changed the title Improved Graph Implementations #8709Improved Graph ImplementationsMay 25, 2023
@nith2001

Copy link
Copy Markdown
ContributorAuthor

Ready for another review!

Comment threadgraphs/graph_adjacency_list.py
@algorithms-keeperalgorithms-keeperBot added the require type hints https://docs.python.org/3/library/typing.html label May 30, 2023

@algorithms-keeperalgorithms-keeperBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Click here to look at the relevant links ⬇️

🔗 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 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.

Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
@algorithms-keeperalgorithms-keeperBot added the tests are failing Do not merge until tests pass label May 30, 2023

@algorithms-keeperalgorithms-keeperBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Click here to look at the relevant links ⬇️

🔗 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 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.

Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
@algorithms-keeperalgorithms-keeperBot removed the tests are failing Do not merge until tests pass label May 30, 2023

@algorithms-keeperalgorithms-keeperBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Click here to look at the relevant links ⬇️

🔗 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 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.

Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py
Comment threadgraphs/graph_adjacency_matrix.py
Comment threadgraphs/graph_adjacency_matrix.py
Comment threadgraphs/graph_adjacency_matrix.py

@algorithms-keeperalgorithms-keeperBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Click here to look at the relevant links ⬇️

🔗 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 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.

Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
@algorithms-keeperalgorithms-keeperBot removed the require type hints https://docs.python.org/3/library/typing.html label May 30, 2023
@nith2001
nith2001 requested a review from cclaussMay 30, 2023 21:53
@nith2001

nith2001 commented May 30, 2023

Copy link
Copy Markdown
ContributorAuthor

Took a few too many commits to resolve the type hints issue and a new ruff issue that popped up about f-strings in exceptions that didn't show up on the local ruff check. As always, appreciate the feedback!

Edit: Still ready for review, the commit below was just a minor fix I needed to make in my error messaging grammar.

@algorithms-keeperalgorithms-keeperBot removed the awaiting reviews This PR is ready to be reviewed label May 31, 2023
@cclauss
cclauss merged commit 4621b0b into TheAlgorithms:masterMay 31, 2023
@nith2001

Copy link
Copy Markdown
ContributorAuthor

Thanks!

@isidroasisidroas mentioned this pull request Jan 25, 2025
14 tasks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@nith2001@cclauss
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); Improved Graph Implementations by nith2001 · Pull Request #8730 · TheAlgorithms/Python · GitHub
Skip to content

Improved Graph Implementations - #8730

Merged
cclauss merged 12 commits into
TheAlgorithms:masterfrom
nith2001:master
May 31, 2023
Merged

Improved Graph Implementations#8730
cclauss merged 12 commits into
TheAlgorithms:masterfrom
nith2001:master

Conversation

@nith2001

@nith2001nith2001 commented May 14, 2023

Copy link
Copy Markdown
Contributor

Describe your change:

The graph implementations using the adjacency list and adjacency matrix were not very comprehensive and lacked supporting functions. I wanted to improve them and also write tests to prove they worked. This also solved Issue #8709, which I brought up. To run my tests, do python3 <testfile>.py under the graphs/tests folder.

  • 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.
    ^^ it changes two, but they're both about the same thing, which is graph implementation. Let me know if that's not okay.
  • 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 include at least one URL that points to Wikipedia or another similar explanation.
  • If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.

Provides new implementation for graph_list.py and graph_matrix.py along with pytest suites for each. FixesTheAlgorithms#8709
@nith2001

nith2001 commented May 14, 2023

Copy link
Copy Markdown
ContributorAuthor

I'll clear up the failed code quality test soon. First-time contributor so I didn't read the Contributor.md as closely as I should've regarding testing and quality.

@algorithms-keeperalgorithms-keeperBot added the tests are failing Do not merge until tests pass label May 14, 2023
Comment threadgraphs/graph_list.py Outdated
@nith2001

nith2001 commented May 15, 2023

Copy link
Copy Markdown
ContributorAuthor

I wrote a bunch of unit tests using the unittest framework rather than doctest because I didn't want to make my code super unnecessarily long. Is that okay?

@algorithms-keeperalgorithms-keeperBot removed the tests are failing Do not merge until tests pass label May 15, 2023
@nith2001

Copy link
Copy Markdown
ContributorAuthor

Not yet, gonna push some comments and docs describing the implementation

@algorithms-keeperalgorithms-keeperBot added the awaiting reviews This PR is ready to be reviewed label May 15, 2023
@nith2001

nith2001 commented May 15, 2023

Copy link
Copy Markdown
ContributorAuthor

Alright, all done. I wanted to store my tests in the graphs/tests folder but I couldn't find a way to import my graph classes without violated some type of black linter code quality. I wanted to do sys.path.append("..") and then import the class but black and ruff wouldn't let me I think. Otherwise, let me know what I can fix!

@nith2001
nith2001 requested a review from cclaussMay 15, 2023 20:59
@nith2001

Copy link
Copy Markdown
ContributorAuthor

Are y'all backlogged with work? Just checking in.

Comment threadgraphs/graph_list.py
Comment threadgraphs/graph_list.py Outdated
Comment threadgraphs/graph_adj_list.py
Comment threadgraphs/graph_adj_list.py Outdated
Comment threadgraphs/graph_adj_list.py Outdated
Comment threadgraphs/graph_adj_list.py Outdated
Comment threadgraphs/graph_adj_list.py Outdated
@nith2001nith2001 changed the title Improved Graph Implementations #8709Improved Graph ImplementationsMay 25, 2023
@nith2001

Copy link
Copy Markdown
ContributorAuthor

Ready for another review!

Comment threadgraphs/graph_adjacency_list.py
@algorithms-keeperalgorithms-keeperBot added the require type hints https://docs.python.org/3/library/typing.html label May 30, 2023

@algorithms-keeperalgorithms-keeperBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Click here to look at the relevant links ⬇️

🔗 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 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.

Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
@algorithms-keeperalgorithms-keeperBot added the tests are failing Do not merge until tests pass label May 30, 2023

@algorithms-keeperalgorithms-keeperBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Click here to look at the relevant links ⬇️

🔗 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 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.

Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py Outdated
@algorithms-keeperalgorithms-keeperBot removed the tests are failing Do not merge until tests pass label May 30, 2023

@algorithms-keeperalgorithms-keeperBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Click here to look at the relevant links ⬇️

🔗 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 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.

Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_matrix.py Outdated
Comment threadgraphs/graph_adjacency_matrix.py
Comment threadgraphs/graph_adjacency_matrix.py
Comment threadgraphs/graph_adjacency_matrix.py
Comment threadgraphs/graph_adjacency_matrix.py

@algorithms-keeperalgorithms-keeperBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Click here to look at the relevant links ⬇️

🔗 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 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.

Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
Comment threadgraphs/graph_adjacency_list.py
@algorithms-keeperalgorithms-keeperBot removed the require type hints https://docs.python.org/3/library/typing.html label May 30, 2023
@nith2001
nith2001 requested a review from cclaussMay 30, 2023 21:53
@nith2001

nith2001 commented May 30, 2023

Copy link
Copy Markdown
ContributorAuthor

Took a few too many commits to resolve the type hints issue and a new ruff issue that popped up about f-strings in exceptions that didn't show up on the local ruff check. As always, appreciate the feedback!

Edit: Still ready for review, the commit below was just a minor fix I needed to make in my error messaging grammar.

@algorithms-keeperalgorithms-keeperBot removed the awaiting reviews This PR is ready to be reviewed label May 31, 2023
@cclauss
cclauss merged commit 4621b0b into TheAlgorithms:masterMay 31, 2023
@nith2001

Copy link
Copy Markdown
ContributorAuthor

Thanks!

@isidroasisidroas mentioned this pull request Jan 25, 2025
14 tasks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@nith2001@cclauss