') + ')', '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('^' + ".*" + ', '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" + ', '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('^' + ".*" + ', '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); } })(); })(); Changed knuth_morris_pratt to be consistent with str.find() by Akashram28 · Pull Request #9079 · TheAlgorithms/Python · GitHub
Skip to content

Changed knuth_morris_pratt to be consistent with str.find() - #9079

Closed
Akashram28 wants to merge 9 commits into
TheAlgorithms:masterfrom
Akashram28:kmp_return_index
Closed

Changed knuth_morris_pratt to be consistent with str.find()#9079
Akashram28 wants to merge 9 commits into
TheAlgorithms:masterfrom
Akashram28:kmp_return_index

Conversation

@Akashram28

@Akashram28Akashram28 commented Sep 23, 2023•
edited
Loading

Copy link
Copy Markdown

Describe your change:

function kmp() now returns the first occurrence of the pattern. If the pattern does not exist in the given text, it returns -1

See #9077

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

Checklist:

  • I have read CONTRIBUTING.md.
  • This pull request is all my own work -- I have not plagiarized.
  • I know that pull requests will not be merged if they fail the automated tests.
  • This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
  • All new Python files are placed inside an existing directory.
  • All filenames are in all lowercase characters with no spaces or dashes.
  • All functions and variable names follow Python naming conventions.
  • All function parameters and return values are annotated with Python type hints.
  • All functions have doctests that pass the automated testing.
  • All new algorithms 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 description above includes the issue number(s) with a closing keyword: "FixesReturning index instead of boolean when knuth_morris_pratt matches. #9077".

@rohan472000

rohan472000 commented Sep 23, 2023•
edited
Loading

Copy link
Copy Markdown
Contributor

Change bool return type to int for kmp function.

@rohan472000

rohan472000 commented Sep 23, 2023

Copy link
Copy Markdown
Contributor

In your PR description, you should tick whatever suits your PR but don't delete other checkboxes, also tick that checkbox for bug fixing or doc change which u deleted.

rohan472000
rohan472000 suggested changes Sep 23, 2023
View reviewed changes
Comment threadstrings/knuth_morris_pratt.py Outdated
Akashram28and others added 2 commits September 23, 2023 16:04
@Akashram28@rohan472000
Update strings/knuth_morris_pratt.py
c3aeabf
Co-authored-by: Rohan Anand <96521078+rohan472000@users.noreply.github.com>
@Akashram28
Changed kmp() to knuth_morris_pratt()
e9babb7
@rohan472000

rohan472000 commented Sep 23, 2023

Copy link
Copy Markdown
Contributor

@cclauss , kindly review it.

@amirsoroush

amirsoroush commented Sep 23, 2023

Copy link
Copy Markdown
Contributor

Thanks. "doctests" are more desirable than print statements. While we're at it, let's write some tests instead.

@amirsoroush

amirsoroush commented Sep 23, 2023•
edited
Loading

Copy link
Copy Markdown
Contributor

Just to note, you can use some keywords in your commit message to link a pull request to an issue, that way if your PR is accepted, the linked issue gets closed automatically. https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue

rohan472000
rohan472000 approved these changes Sep 23, 2023
View reviewed changes

@rohan472000rohan472000 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@algorithms-keeperalgorithms-keeperBot added awaiting reviews This PR is ready to be reviewed tests are failing Do not merge until tests pass labels Sep 23, 2023
@cclauss

cclauss commented Sep 23, 2023•
edited
Loading

Copy link
Copy Markdown
Member

I added doctests that fail...

=================================== FAILURES ===================================
___________ [doctest] strings.knuth_morris_pratt.knuth_morris_pratt ____________
009 1) Preprocess pattern to identify any suffixes that are identical to prefixes
010 011 This tells us where to continue from if we get a mismatch between a character
012 in our pattern and the text.
013 014 2) Step through the text one character at a time and compare it to a character in
015 the pattern updating our location within the pattern if necessary
016 017 >>> kmp = "knuth_morris_pratt"
018 >>> knuth_morris_pratt(kmp, "kn") == kmp.find("kn")
Expected:
True
Got:
False

As discussed in CONTRIBUTING.md, you can run these on your own computer with:
python3 -m doctest -v strings/knuth_morris_pratt.py

@rohan472000

rohan472000 commented Sep 23, 2023

Copy link
Copy Markdown
Contributor

while testing the doctests added by @cclauss with existing code of knuth_morris_pratt.py in repo, it is also failing.

@Akashram28

Akashram28 commented Sep 23, 2023•
edited
Loading

Copy link
Copy Markdown
Author

image

knuth_morris_pratt The pattern is the first parameter, the text is the second. Or am I doing something wrong ? Sry this is my first PR

@Akashram28Akashram28 reopened this Sep 23, 2023
@algorithms-keeperalgorithms-keeperBot added the enhancement This PR modified some existing files label Sep 23, 2023
@cclauss

cclauss commented Sep 24, 2023•
edited
Loading

Copy link
Copy Markdown
Member

If we are going to be consistent with str.find() then we should change knuth_morris_pratt(sub, text) to knuth_morris_pratt(text, sub) to match the str.find() calling conventions and knuth_morris_pratt(text, sub) == text.find(sub) should always be True.

@rohan472000rohan472000 mentioned this pull request Sep 25, 2023
Merged
14 tasks
@cclausscclauss closed this Sep 30, 2023
@Akashram28
Akashram28 deleted the kmp_return_index branch October 1, 2023 11:54
@Akashram28
Akashram28 restored the kmp_return_index branch October 1, 2023 11:54
@Akashram28
Akashram28 deleted the kmp_return_index branch October 1, 2023 11:54
@Akashram28
Akashram28 restored the kmp_return_index branch October 1, 2023 11:54
@Akashram28
Akashram28 deleted the kmp_return_index branch October 1, 2023 11:54
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@rohan472000rohan472000rohan472000 approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

awaiting reviewsThis PR is ready to be reviewedenhancementThis PR modified some existing filestests are failingDo not merge until tests pass

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Returning index instead of boolean when knuth_morris_pratt matches.

4 participants

@Akashram28@rohan472000@amirsoroush@cclauss