Skip to content

Conversation

LazarusCoder
Copy link

@LazarusCoder LazarusCoder commented Dec 29, 2020

References to other Issues or PRs or Relevant literature

Fixes #308 #310

Brief description of what is fixed or changed

Longest Common Subsequence #308 is been added with backtracking
so now we get the Lenght of LCS and sequence too

Other comments

@czgdp1807
Copy link
Member

Please go through some already present code in master. The code is okay but the function names aren't descriptive, the document string isn't following the already existing style. Code quality checks are failing.

@codecov
Copy link

codecov bot commented Jan 10, 2021

Codecov Report

Merging #310 (9643882) into master (42832f7) will increase coverage by 0.008%.
The diff coverage is 100.000%.

@@              Coverage Diff              @@
##            master      #310       +/-   ##
=============================================
+ Coverage   98.866%   98.874%   +0.008%     
=============================================
  Files           25        25               
  Lines         2999      3021       +22     
=============================================
+ Hits          2965      2987       +22     
  Misses          34        34               
Impacted Files Coverage Δ
pydatastructs/linear_data_structures/__init__.py 100.000% <ø> (ø)
pydatastructs/linear_data_structures/algorithms.py 99.507% <100.000%> (+0.059%) ⬆️

Impacted file tree graph

@LazarusCoder
Copy link
Author

LCS with Back Tracking Done

@sidhu1012
Copy link

Please go through some already present code in master. The code is okay but the function names aren't descriptive, the document string isn't following the already existing style.

These issues still persist in code. Please make function name more informative and follow numpy docstring guidelines.

@LazarusCoder
Copy link
Author

LazarusCoder commented Jan 10, 2021

Please go through some already present code in master. The code is okay but the function names aren't descriptive, the document string isn't following the already existing style.

These issues still persist in code. Please make function name more informative and follow numpy docstring guidelines.
@czgdp1807
right now function name is LCS =>Longest common sub sequence would be big as a function name same while calling it dont you think?

@@ -546,3 +547,54 @@ def bucket_sort(array: Array, **kwargs) -> Array:
if _check_type(array, DynamicArray):
array._modify(force=True)
return array

def LCS(data_A,data_B):
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
def LCS(data_A,data_B):
def lowest_common_subsequence(seq1, seq2):

@czgdp1807
Copy link
Member

Care to add some spaces around =, after, ,. It just makes it easier to read code.

@czgdp1807 czgdp1807 mentioned this pull request Jan 13, 2021
@czgdp1807 czgdp1807 changed the title LCS with Backtracking [WIP] Added Longest Common Sequence Jan 13, 2021
@sidhu1012
Copy link

It has conflicts now. Please resolve them.

@czgdp1807
Copy link
Member

Closing in favour of #315

@czgdp1807 czgdp1807 closed this Jan 17, 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.

Longest common subsequence
4 participants