Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

feat: python impl for 0-1 BFS#1336

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
clumsy wants to merge5 commits intocp-algorithms:mainfromclumsy:master
Closed

Conversation

clumsy
Copy link
Contributor

Adding python implementation, fixing a minor grammatical error.

@adamant-pwn
Copy link
Member

adamant-pwn commentedOct 11, 2024
edited
Loading

Thanks for the pull request! Could you please rebase it on the current master branch, so that workflows run correctly? Also I noticed that you have provided an implementation of Dijkstra algorithm usingset, which unfortunately wouldn't work because Python's sets are based on hash-tables, and as such don't produce the smallest element.

What you actually want here is probablyheapq, could you please update the pull request using it? Also, did you check your code on any specific problems?

@clumsy
Copy link
ContributorAuthor

clumsy commentedOct 13, 2024
edited
Loading

Thanks for the pull request! Could you please rebase it on the current master branch, so that workflows run correctly? Also I noticed that you have provided an implementation of Dijkstra algorithm usingset, which unfortunately wouldn't work because Python's sets are based on hash-tables, and as such don't product the smallest element.

What you actually want here is probablyheapq, could you please update the pull request using it? Also, did you check your code on any specific problems?

Sure, tried to follow the C++ original as much as possible. It worked for several test cases, but I'll switch toheapq and rebase.

@wqweto
Copy link

@clumsy Please don't indent codeblock's backticks in markdown i.e. make sure```cpp stays at the beginning on the line.

@adamant-pwn
Copy link
Member

adamant-pwn commentedOct 13, 2024
edited
Loading

Indenting here is okay, because it uses Material'scontent tabs feature.

@clumsy
Copy link
ContributorAuthor

Indenting here is okay, because it uses Material'scontent tabs feature.

Exactly, without this change there are no C++/Python tabs to switch from. This follows the same format as other pages with examples in two languages.

@jxu
Copy link
Contributor

jxu commentedOct 14, 2024

Personally I think there should only be one implementation, and a good reason to have multiple. If we have python as pseudocode, then it shouldn't be hidden in a content tab. My 2c

@adamant-pwn
Copy link
Member

I'm generally fine with having both C++ and Python. The latter is more beginner-friendly, mostly simple to review and can also be used in problems where performance doesn't matter as much, e.g. for Project Euler problems, which is also the reason I've included both incontinued fractions article (well, and also because with continued fractions, you are much more likely to require big integers). I think it's fine to use it as a second default, when available. It doesn't really take anything from article's quality, and can be useful for some people.

Hiding it in content tabs is, IMO, fine because the majority of CPers still use C++, and using tabs allows to preserve article space, so that only people for whom Python code would actually be useful will actually open it.

clumsy reacted with thumbs up emoji

@clumsy
Copy link
ContributorAuthor

+1 to what@adamant-pwn said.
Also, I've addressed your comment, please let me know if there's more to edit.

@adamant-pwnadamant-pwn deleted the branchcp-algorithms:mainOctober 14, 2024 18:52
@adamant-pwnadamant-pwn changed the base branch frommaster tomainOctober 14, 2024 19:18
@github-actionsGitHub Actions
Copy link
Contributor

Preview the changes for PR#1336 athttps://gh.cp-algorithms.com/1336/ (current version:9260362).

@clumsy
Copy link
ContributorAuthor

The base branch changed, will cut another PR.

@clumsyclumsy closed thisOct 24, 2024
github-actionsbot added a commit that referenced this pull requestOct 24, 2024
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@clumsy@adamant-pwn@wqweto@jxu

[8]ページ先頭

©2009-2025 Movatter.jp