Skip to content
This repository has been archived by the owner on Apr 29, 2020. It is now read-only.

Added Knuth Morris Prat (KMP) #73

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Added Knuth Morris Prat (KMP) #73

wants to merge 2 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Apr 24, 2016

Added implementation of Knuth Morris Prat (KMP) algorithm for string comparison.
--Team Code-ologists

@CuriousLearner
Copy link
Member

CuriousLearner commented Apr 24, 2016 •
edited

@anikamurarka Please set up your git config properly. Your email is not verified.

Also the checks have failed. Please rectify them.

@ghost
Copy link
Author

ghost commented Apr 25, 2016

Thanks @CuriousLearner, will make the required changes.

@goelakash
Copy link
Collaborator

@anikamurarka Few things:

  1. Add reference for the implementation.
  2. Add tests.
  3. Add docs.

@ghost
Copy link
Author

ghost commented May 3, 2016

Thanks @goelakash ... will update it soon

@aktech
Copy link
Member

aktech commented May 13, 2016 •
edited

@anikamurarka Please make the following changes.

@ghost
Copy link
Author

ghost commented May 14, 2016

@goelakash please review !!

rahulroxx
rahulroxx reviewed May 15, 2016
View reviewed changes
pydsa/__init__.py
@@ -10,5 +10,7 @@
from .linear_search import linear_search
from .queue import queue
from .counting_sort import counting_sort
from .stack import Stack
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this redundant import

@ghost
Copy link
Author

ghost commented May 15, 2016

@goelakash @rahulroxx please review !!

goelakash
goelakash reviewed May 15, 2016
View reviewed changes
pydsa/tests/test_kmp.py
def test_bubble_sort():
a = 'BBC ABCDAB ABCDABCDABDE'
b = 'ABCDABD'
assert kmp(a, b) == 15
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't hard code this. Instead try kmp(a, b) == a.find(b)

@goelakash
Copy link
Collaborator

@anikamurarka Once the changes are done, please squash the commits as well.

@ghost
Copy link
Author

ghost commented May 25, 2016

@goelakash, I have made the changes, please review !

@goelakash
Copy link
Collaborator

@anikamurarka You have made some error in rebasing the branch. As a result, you are re-commiting commits of other contributors (see your git log).

  1. I suggest you do git rebase -i master and drop those commits that don't belong to you. (replace pick with drop).
  2. Also, squash all your own commits in a single commit after that.

anika murarka and others added 2 commits June 3, 2016 17:15
@a2batic
Squashing
da4d66b 
Added Knuth Morris Prat (KMP)

Commits Squashed

Changes

Changes
@a2batic
Merge conglict
578def2
@CuriousLearner
Copy link
Member

@a2batic @ghost are you still on this?

@tapaswenipathak
Copy link

#12.

@a2batic
Copy link
Contributor

a2batic commented Sep 1, 2016

We resolved all the errors in commit no. -> 578def2. @CuriousLearner

goelakash
goelakash reviewed Sep 4, 2016
View reviewed changes
pydsa/kmp.py

def kmp(full_string, pattern):
'''
It takes two arguments, the main string and the patterb which is to be
Copy link
Collaborator

Choose a reason for hiding this comment

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

patterb -> pattern

@aktech
Copy link
Member

aktech commented Sep 4, 2016

It seems the proposer has deleted his/her Github account.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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.

None yet
6 participants
@CuriousLearner @goelakash @aktech @tapaswenipathak @a2batic @rahulroxx