Skip to content

Added a function to check ordering in ODA and DODA #344

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

Merged
merged 10 commits into from
Mar 19, 2021

Conversation

Nora2412
Copy link
Contributor

Appended algorithms file with is_sorted function. This will be useful to
check whether the array given is sorted or not.

References to other Issues or PRs or Relevant literature

Brief description of what is fixed or changed

Other comments

OneDimensionalArray.

Appended algorithms file with is_sorted function. This will be useful to
check whether the array given is sorted or not.
@codecov
Copy link

codecov bot commented Mar 11, 2021

Codecov Report

Merging #344 (8730d80) into master (a21d00e) will increase coverage by 0.010%.
The diff coverage is 100.000%.

@@              Coverage Diff              @@
##            master      #344       +/-   ##
=============================================
+ Coverage   98.550%   98.561%   +0.010%     
=============================================
  Files           25        25               
  Lines         3243      3267       +24     
=============================================
+ Hits          3196      3220       +24     
  Misses          47        47               
Impacted Files Coverage Δ
pydatastructs/linear_data_structures/__init__.py 100.000% <ø> (ø)
pydatastructs/linear_data_structures/algorithms.py 99.613% <100.000%> (+0.015%) ⬆️
...ucts/miscellaneous_data_structures/disjoint_set.py 100.000% <0.000%> (ø)

Impacted file tree graph

Comment on lines 811 to 812
>>> arr = OneDimensionalArray(int,[4,3,2,1])
>>> is_sorted(arr,0,3)
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
>>> arr = OneDimensionalArray(int,[4,3,2,1])
>>> is_sorted(arr,0,3)
>>> arr = OneDimensionalArray(int, [4, 3, 2, 1])
>>> is_sorted(arr, 0, 3)

Make suggested changes wherever appicable. Add a space after comma

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.

Please apply this changes wherever required

Comment on lines 819 to 820
for i in range(start+1,end+1):
if(array[i]<array[i-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
for i in range(start+1,end+1):
if(array[i]<array[i-1]):
for i in range(start + 1, end + 1):
if(array[i] < array[i - 1]):

@czgdp1807
Copy link
Member

This is a bit trivial. I mean checking if an array sorted or not in O(N) time isn't really helpful. For keeping time complexity constant we may need to maintain a state inside ODA class to track when the sort function was called on an array object. More or less it would be just like maintaining the history of the array. We can have this feature as something extra.


"""
for i in range(start+1,end+1):
if(array[i]<array[i-1]):
Copy link
Member

Choose a reason for hiding this comment

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

This will not work with DODA (the dynamic arrays). An operator is defined in _comp (or something similar) in misc_util.py. Please use that here.
Moreover, we should allow a custom comparator (defaulting to <=) to check if an array is ordered according to some criteria specified by the passed comparator. The function can also be renamed to is_ordered.

@Smit-create
Copy link
Member

For keeping time complexity constant we may need to maintain a state inside ODA class to track when the sort function was called on an array object

What if the user has'nt call sort function and has already given an sorted array as input?

This is a bit trivial. I mean checking if an array sorted or not in O(N) time isn't really helpful.

Presently, C++, also uses linear time complexity to check sortedness: http://www.cplusplus.com/reference/algorithm/is_sorted/

An operator is defined in _comp (or something similar) in misc_util.py. Please use that here.

This is something that would be good to have instead of using operators directly

@Smit-create
Copy link
Member

Smit-create commented Mar 12, 2021

Taking references from https://en.cppreference.com/w/cpp/algorithm/lower_bound and https://en.cppreference.com/w/cpp/algorithm/is_sorted_until:
We could do something like this:

def is_sorted(array, start, end, cmp=None):
    # cmp is a function that takes (a, b) and returns true if a is strictly less than b (a<b)
    if cmp is None:
         cmp = lambda a, b: (a < b) # we will use python defined <
    # algo
    return True/False

Similar thing can be done on #351
What do you think @czgdp1807 ?

@@ -787,3 +788,35 @@ def longest_common_subsequence(seq1: OneDimensionalArray, seq2: OneDimensionalAr
check_mat[i][j] = check_mat[i][j-1]

return OneDimensionalArray(seq1._dtype, check_mat[row][col][-1])

def is_sorted(array, start, end):
Copy link
Member

Choose a reason for hiding this comment

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

Please rename this function to is_ordered and accept an extra parameter comp.

@czgdp1807
Copy link
Member

What do you think @czgdp1807 ?

We should add is_ordered as it is more generic and allows more flexibility.

Comment on lines 818 to 821
"""
for i in range(start + 1, end + 1):
if(array[i] < array[i - 1]):
return False
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
"""
for i in range(start + 1, end + 1):
if(array[i] < array[i - 1]):
return False
"""
if comp is None:
comp = lambda a, b: a < b
for i in range(start + 1, end + 1):
if(comp(array[i], array[i - 1])):
return False

Comment on lines 159 to 161
arr4 = ODA(int, [1, 2, 3, 4, 5, 6, 7, 8, 9, 10], None)
output = is_ordered(arr4, 0, 9)
assert output == expected_result
Copy link
Member

Choose a reason for hiding this comment

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

Also add test cases for custom defined comp

@Smit-create
Copy link
Member

@Nora2412 Please use python -m pytest to check locally which test is failing (presently due to trailing space/tab on some line)

@Smit-create
Copy link
Member

Smit-create commented Mar 15, 2021

If you have trailing whitespace it will show errors. This one will fix unwanted spaces:
$ ./bin/strip_whitespace <file>

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. Please address few suggestions

Comment on lines 135 to 137
arr = ODA(int,[1,2,5,6])
output = is_sorted(arr,0,len(arr)-1)
assert output == expected_result
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
arr = ODA(int,[1,2,5,6])
output = is_sorted(arr,0,len(arr)-1)
assert output == expected_result
arr = ODA(int, [1, 2, 5, 6])
output = is_sorted(arr, 0, len(arr) - 1)
assert output == expected_result

Apply this change whereever necessary

Comment on lines 159 to 161
arr4 = ODA(int,[1,2,3,4,5,6,7,8,9,10])
output = is_sorted(arr4,0,9)
assert output == expected_result
Copy link
Member

Choose a reason for hiding this comment

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

Also add tests for user-defined comp

@Nora2412
Copy link
Contributor Author

@Smit-create can you please review and let me know if any further changes are required.

@czgdp1807 czgdp1807 changed the title Add a function to check sorting of One Dimensional Array Added a function to check ordering in ODA and DODA Mar 19, 2021
@czgdp1807
Copy link
Member

Will merge once the tests pass.

@czgdp1807 czgdp1807 merged commit 5488001 into codezonediitj:master Mar 19, 2021
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