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.

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

upper_bound and lower_bound functions added #351

Merged
merged 14 commits into from
Mar 27, 2021

Conversation

sHiVaNgI821
Copy link
Contributor

References to other Issues or PRs or Relevant literature

Added the upper_bound and lower_bound functions to the algorithms library. Upper_bound returns the index of the first occurence of an element greater than value, in the given sorted OneDimensionalArray. Lower_bound returns the index of the first occurence of an element which is not less than value, in the given OneDimensionalArray.

Brief description of what is fixed or changed

Other comments

@codecov
Copy link

codecov bot commented Mar 12, 2021

Codecov Report

Merging #351 (c676829) into master (e1134b1) will increase coverage by 0.013%.
The diff coverage is 100.000%.

@@              Coverage Diff              @@
##            master      #351       +/-   ##
=============================================
+ Coverage   98.561%   98.574%   +0.013%     
=============================================
  Files           25        25               
  Lines         3267      3297       +30     
=============================================
+ Hits          3220      3250       +30     
  Misses          47        47               
Impacted Files Coverage Δ
pydatastructs/linear_data_structures/__init__.py 100.000% <ø> (ø)
pydatastructs/linear_data_structures/algorithms.py 99.653% <100.000%> (+0.040%) ⬆️

Impacted file tree graph

@Smit-create
Copy link
Member

@sHiVaNgI821 Please address this #344 (comment) in your PR too

@Smit-create Smit-create added medium and removed easy labels Mar 13, 2021
Comment on lines 821 to 832
>>> arr = ODA(int, [7, 6, 5, 5, 4])
>>> upperBound = upper_bound(arr, 0, 4, 5, lambda x, y: x >= y)
>>> upperBound
4

Note
====

The OneDimensionalArray must be sorted beforehand
"""
if comp is None:
comp = lambda a, b: (a <= b)
Copy link
Member

Choose a reason for hiding this comment

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

The order needs to be well defined. i.e, strict inequality is followed in order. You can have a look at the implementation here: https://en.cppreference.com/w/cpp/algorithm/upper_bound.

Comment on lines 837 to 838
mid = (start+end)//2
if comp(array[mid],value):
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
mid = (start+end)//2
if comp(array[mid],value):
mid = (start + end)//2
if comp(array[mid], value):

Comment on lines 847 to 858
Finds the the index of the first occurence of an element which is not less than value according to an order defined, in the given OneDimensionalArray

Parameters
========
array: OneDimensionalArray
The sorted array (sorted according to a custom comparator function) in which the lower bound has to be found

start: int
The staring index of the portion of the array in which the lower bound of a given value has to be looked for

end: int
The ending index of the portion of the array in which the lower bound of a given value has to be looked for
Copy link
Member

Choose a reason for hiding this comment

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

The line width should not exceed 79 characters. Wrap up text in multiple lines not more than 79 chars

Comment on lines 863 to 864
output: int
Lower bound of the given value in the given sorted OneDimensionalArray
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
output: int
Lower bound of the given value in the given sorted OneDimensionalArray
output: int
Lower bound of the given value in the given sorted OneDimensionalArray

Comment on lines 890 to 891
mid = (start+end)//2
if comp(array[mid],value):
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
mid = (start+end)//2
if comp(array[mid],value):
mid = (start + end)//2
if comp(array[mid], value):

start = mid + 1
else:
index = mid
end = mid -1
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
end = mid -1
end = mid - 1

Copy link
Member

@Smit-create Smit-create left a comment

Choose a reason for hiding this comment

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

This looks good. Just have a look at some suggestions.

Comment on lines 815 to 816
output: int
Upper bound of the given value in the given sorted OneDimensionalArray
Copy link
Member

Choose a reason for hiding this comment

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

Index of upper bound of given value...

Comment on lines 871 to 872
output: int
Lower bound of the given value in the given sorted OneDimensionalArray
Copy link
Member

Choose a reason for hiding this comment

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

Same

Comment on lines 134 to 136
output = upper_bound(arr1, 0, len(arr1) - 1, 3, None)
expected_result = 2
assert expected_result == output
Copy link
Member

Choose a reason for hiding this comment

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

From, here I think there needs a slight change in the algo. That is output 2 isn't correct mathematically for this in the given range. Hence, we should consider the interval as [start, end). And so, I expect the output of it as 3. (You can verify it by yourself, we are making it similar to cpp)

Copy link
Member

Choose a reason for hiding this comment

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

Also, after making this interval change, please add tests apart from [0, len(arr)) interval too.

@Smit-create
Copy link
Member

Looks good to me.

@Smit-create
Copy link
Member

@czgdp1807 Have a look at it

@Smit-create
Copy link
Member

@sHiVaNgI821 Please resolve merge conflicts

Copy link
Member

@Smit-create Smit-create left a comment

Choose a reason for hiding this comment

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

Apply these suggestions to both functions. Apart from this, it looks good to me.

Comment on lines 860 to 866
start: int
The staring index of the portion of the array in which the upper bound
of a given value has to be looked for

end: int
The ending index of the portion of the array in which the upper bound
of a given value has to be looked for
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
start: int
The staring index of the portion of the array in which the upper bound
of a given value has to be looked for
end: int
The ending index of the portion of the array in which the upper bound
of a given value has to be looked for
start: int, optional.
The staring index of the portion of the array in which the upper bound
of a given value has to be looked for. By default is 0.
end: int, optional
The ending index of the portion of the array in which the upper bound
of a given value has to be looked for. By default is len(array)
comp: boolean funtion, optional
A function that takes two paramters and returns True if 1st parameter is strictly
smaller than 2nd parameter

Comment on lines 894 to 895
# if comp is None:
# comp = lambda a, b: (a < b)
Copy link
Member

Choose a reason for hiding this comment

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

Remove this comment

@czgdp1807
Copy link
Member

Looks Good. Will polish it and push back again.

@Smit-create
Copy link
Member

@czgdp1807 This can be merged.

@czgdp1807 czgdp1807 merged commit 80067d3 into codezonediitj:master Mar 27, 2021
@czgdp1807
Copy link
Member

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants