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

Added Suffix Tree implementation using Ukkonen algorithm #524

Open
wants to merge 35 commits into
base: main
Choose a base branch
from

Conversation

CarolLuca
Copy link
Contributor

References to other Issues or PRs or Relevant literature

Brief description of what is fixed or changed

Added suffix tree class

Other comments

May be considered as GSoC 23 work sample

@czgdp1807
Copy link
Member

I would recommend to always shift to a new branch (other than main before making a new PR). Right now you have made your PR using your main branch. Keep this PR as is but from next time onwards update your main locally and then switch to a new branch for a new feature/bug fix.

Copy link
Member

@czgdp1807 czgdp1807 left a comment

Choose a reason for hiding this comment

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

Thanks for this. I have left some reviews. Please address them.

pydatastructs/strings/tests/test_suffix_tree.py Outdated Show resolved Hide resolved
pydatastructs/strings/suffix_tree.py Outdated Show resolved Hide resolved
pydatastructs/strings/suffix_tree.py Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Apr 4, 2023

Codecov Report

Merging #524 (b8c6b45) into main (2482adb) will increase coverage by 0.045%.
The diff coverage is 100.000%.

Additional details and impacted files
@@              Coverage Diff              @@
##              main      #524       +/-   ##
=============================================
+ Coverage   98.558%   98.603%   +0.045%     
=============================================
  Files           32        33        +1     
  Lines         4092      4226      +134     
=============================================
+ Hits          4033      4167      +134     
  Misses          59        59               
Impacted Files Coverage Δ
pydatastructs/strings/__init__.py 100.000% <100.000%> (ø)
pydatastructs/strings/suffix_tree.py 100.000% <100.000%> (ø)
pydatastructs/utils/misc_util.py 99.250% <100.000%> (+0.120%) ⬆️

Impacted file tree graph

@czgdp1807
Copy link
Member

See, https://app.codecov.io/gh/codezonediitj/pydatastructs/pull/524/blob/pydatastructs/strings/suffix_tree.py. There are lots of lines (see the red ones) which aren't tested.

Also add SuffixTree to the following list,

pyds.graphs.adjacency_list.AdjacencyList,
pyds.graphs.adjacency_matrix.AdjacencyMatrix,
pyds.DoublyLinkedList, pyds.SinglyLinkedList,
pyds.SinglyCircularLinkedList,
pyds.DoublyCircularLinkedList,
pyds.OneDimensionalArray, pyds.MultiDimensionalArray,
pyds.DynamicOneDimensionalArray,
pyds.trees.BinaryTree, pyds.BinarySearchTree,
pyds.AVLTree, pyds.SplayTree, pyds.BinaryTreeTraversal,
pyds.DHeap, pyds.BinaryHeap, pyds.TernaryHeap, pyds.BinomialHeap,
pyds.MAryTree, pyds.OneDimensionalSegmentTree,
pyds.Queue, pyds.miscellaneous_data_structures.queue.ArrayQueue,
pyds.miscellaneous_data_structures.queue.LinkedListQueue,
pyds.PriorityQueue,
pyds.miscellaneous_data_structures.queue.LinkedListPriorityQueue,
pyds.miscellaneous_data_structures.queue.BinaryHeapPriorityQueue,
pyds.miscellaneous_data_structures.queue.BinomialHeapPriorityQueue,
pyds.Stack, pyds.miscellaneous_data_structures.stack.ArrayStack,
pyds.miscellaneous_data_structures.stack.LinkedListStack,
pyds.DisjointSetForest, pyds.BinomialTree, pyds.TreeNode, pyds.MAryTreeNode,
pyds.LinkedListNode, pyds.BinomialTreeNode, pyds.AdjacencyListGraphNode,
pyds.AdjacencyMatrixGraphNode, pyds.GraphEdge, pyds.Set, pyds.BinaryIndexedTree,
pyds.CartesianTree, pyds.CartesianTreeNode, pyds.Treap, pyds.RedBlackTreeNode, pyds.RedBlackTree,
pyds.Trie, pyds.TrieNode, pyds.SkipList, pyds.RangeQueryStatic, pyds.RangeQueryDynamic, pyds.SparseTable,
pyds.miscellaneous_data_structures.segment_tree.OneDimensionalArraySegmentTree,
pyds.bubble_sort, pyds.linear_search, pyds.binary_search, pyds.jump_search,
pyds.selection_sort, pyds.insertion_sort, pyds.quick_sort]

@czgdp1807 czgdp1807 marked this pull request as draft April 4, 2023 17:32
pydatastructs/strings/suffix_tree.py Outdated Show resolved Hide resolved
@CarolLuca CarolLuca marked this pull request as ready for review April 8, 2023 09:39
@CarolLuca
Copy link
Contributor Author

I followed all the requested changes and added more tests such that the testing covers all the methods and sub-cases. I would be grateful if you could tell me if I can polish the classes in any other way.

@CarolLuca CarolLuca requested a review from czgdp1807 April 8, 2023 11:10
Copy link
Member

@czgdp1807 czgdp1807 left a comment

Choose a reason for hiding this comment

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

Thanks for this. Sorry for the later review. Mostly related to documentation.

pydatastructs/strings/suffix_tree.py Outdated Show resolved Hide resolved
pydatastructs/strings/suffix_tree.py Outdated Show resolved Hide resolved
pydatastructs/strings/suffix_tree.py Outdated Show resolved Hide resolved
pydatastructs/strings/suffix_tree.py Outdated Show resolved Hide resolved
@czgdp1807 czgdp1807 marked this pull request as draft April 12, 2023 16:42
@CarolLuca CarolLuca marked this pull request as ready for review April 19, 2023 19:17
@CarolLuca
Copy link
Contributor Author

Thanks for this. Sorry for the later review. Mostly related to documentation.

I combined the __new__ and __init__ methods successfully, and added documentation for the main class, but also for the auxiliary classes from misc_util.py

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

Successfully merging this pull request may close these issues.

2 participants