Skip to content
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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

Implemented search algorithms and their test #73

Merged
merged 1 commit into from
Oct 7, 2020
Merged

Implemented search algorithms and their test #73

merged 1 commit into from
Oct 7, 2020

Conversation

devta108
Copy link
Contributor

@devta108 devta108 commented Oct 5, 2020 •
edited
Loading

Description

Implemented different search algorithms and their respective tests as follows

  1. Linear Search
  2. Binary Search
  3. Jump Search
  4. Interpolation Search
  5. Fibonacci Search

Related Issue

** Please don't create a Pull request without creating an issue first **

Fixes #23

Type of Change:

  • Code
  • Quality Assurance
  • Documentation

Code/Quality Assurance Only

  • Bug fix (non-breaking change which fixes an issue)
  • This change requires a documentation update (software upgrade on readme file)

Motivation and Context

How Has This Been Tested?

Unittest

Checklist:

  • I read the CONTRIBUTING document before making this pull request
  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code or materials
  • I have commented my code or provided relevant documentation, particularly in hard-to-understand areas
  • My change requires change to documentation
  • I have updated documentation accordingly
  • I have updated the requirements file
  • My changes generate no new warnings
  • My PR currently breaks something (fix or feature that would cause existing functionality to not work as expected)
  • I have added tests to cover my changes
  • All new and existing tests passed

Screenshots (if appropriate):

codesankalp
codesankalp requested changes Oct 5, 2020
View reviewed changes
Copy link
Owner

@codesankalp codesankalp left a comment

Choose a reason for hiding this comment

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

You have to link your issue with pull request.
Try to do the mentioned changes.

README.md Outdated Show resolved Hide resolved
dsalgo/search.py Outdated Show resolved Hide resolved
dsalgo/search.py Outdated Show resolved Hide resolved
dsalgo/search.py Show resolved Hide resolved
dsalgo/search.py Outdated Show resolved Hide resolved
dsalgo/search.py Outdated Show resolved Hide resolved
dsalgo/search.py Outdated Show resolved Hide resolved
dsalgo/search.py Show resolved Hide resolved
@codesankalp
Copy link
Owner

also this branch have conflicts so rebase your branch with master,
try to squash commit into one

@codesankalp
Copy link
Owner

@nisheksharma after 2 days if you don't change anything then it will be marked as invalid.

@devta108 devta108 force-pushed the search branch 2 times, most recently from bec9780 to 0bd2cc4 Compare October 7, 2020 10:06
@devta108 devta108 changed the title Search Implemented search algorithms and their test Oct 7, 2020
@devta108
Copy link
Contributor Author

devta108 commented Oct 7, 2020

@codesankalp review

codesankalp
codesankalp approved these changes Oct 7, 2020
View reviewed changes
Copy link
Owner

@codesankalp codesankalp left a comment

Choose a reason for hiding this comment

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

Tested all the code and works properly.
Code styling is also good.
It is ready to merge.

@codesankalp
Copy link
Owner

@Aryamanz29
@17sushmita
please review this pr.

devkapilbansal
devkapilbansal requested changes Oct 7, 2020
View reviewed changes
Copy link
Collaborator

@devkapilbansal devkapilbansal left a comment

Choose a reason for hiding this comment

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

@nisheksharma rename your file to test_search

@devkapilbansal
Copy link
Collaborator

Also, please follow commit message guidelines

@devkapilbansal
Copy link
Collaborator

Pinging @17sushmita for testing code locally before merge

@devkapilbansal devkapilbansal added the hacktoberfest-accepted Accept for hacktoberfest - will merge later label Oct 7, 2020
@devkapilbansal
Copy link
Collaborator

Thanks @nisheksharma just one last request from my side. Change type to feature instead of file, as file is not a commit type defined in our guidelines

@devta108
Copy link
Contributor Author

devta108 commented Oct 7, 2020

@nisheksharma rename your file to test_search

Done

@devta108
Copy link
Contributor Author

devta108 commented Oct 7, 2020

Thanks @nisheksharma just one last request from my side. Change type to feature instead of file, as file is not a commit type defined in our guidelines

done

@devta108 @devkapilbansal
[feature] Implemented search algorithms and their test codesankalp#23
1a6e295 
Following algorithms are implemented with their tests:
	1) Linear search
	2) Binary search
	3) Jump search
	4) Interpolation search
	5) Fibonacci search

Renamed test files and removed driver code

Fixes codesankalp#23

Signed off by:- @devkapilbansal
devkapilbansal
devkapilbansal approved these changes Oct 7, 2020
View reviewed changes
Copy link
Collaborator

@devkapilbansal devkapilbansal left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@nishesksharma thanks Made some changes. Hope you don't mind

@devkapilbansal
Copy link
Collaborator

LGTM 👍 Thanks @nisheksharma

@devkapilbansal devkapilbansal removed Status: Review Required hacktoberfest-accepted Accept for hacktoberfest - will merge later labels Oct 7, 2020
@devkapilbansal devkapilbansal merged commit 739de8f into codesankalp:master Oct 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@devkapilbansal devkapilbansal devkapilbansal approved these changes

@codesankalp codesankalp codesankalp approved these changes

@17sushmita 17sushmita Awaiting requested review from 17sushmita

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement searching algorithms
3 participants
@devta108 @codesankalp @devkapilbansal