-
Notifications
You must be signed in to change notification settings - Fork 313
Added LCA and tests #88
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
Conversation
|
Do not close this PR. Just push the updates in |
pydatastructs/trees/binary_trees.py
Outdated
| curr_root = self.tree[curr_root].right | ||
| if curr_root == u or curr_root == v: | ||
| return curr_root | ||
| u_left = self.comparator(self.tree[u].key, self.tree[curr_root].key) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please split this into two lines. Doesn't look clean.
pydatastructs/trees/binary_trees.py
Outdated
| u, v = self.search(j), self.search(k) | ||
| if (u is None) or (v is None): | ||
| return None | ||
| u_left = self.comparator(self.tree[u].key, self.tree[curr_root].key) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
pydatastructs/trees/binary_trees.py
Outdated
| if (u is None) or (v is None): | ||
| return None | ||
| u_left = self.comparator(self.tree[u].key, self.tree[curr_root].key) | ||
| v_left = self.comparator(self.tree[v].key, self.tree[curr_root].key) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
pydatastructs/trees/binary_trees.py
Outdated
| if curr_root == u or curr_root == v: | ||
| return curr_root | ||
| u_left = self.comparator(self.tree[u].key, self.tree[curr_root].key) | ||
| v_left = self.comparator(self.tree[v].key, self.tree[curr_root].key) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
pydatastructs/trees/binary_trees.py
Outdated
| Parameter | ||
| ========= | ||
| key: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
key: Python object
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't addressed.
pydatastructs/trees/binary_trees.py
Outdated
| key: | ||
| node to be searched | ||
| path: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
path: list
Though I don't think it is needed in the API.
pydatastructs/trees/binary_trees.py
Outdated
| if self._simple_path(key, self.tree[root].left, path) or \ | ||
| self._simple_path(key, self.tree[root].right, path): | ||
| return True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please convert this into iterative logic using pydatastructs.Stack as Python has a finite recursion limit irrespective of the size of heap memory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stack hasn't been implemented yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I made all the changes you have mentioned except this.
pydatastruct.stack is still abstract.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See, https://github.com/codezonediitj/pydatastructs/blob/master/pydatastructs/miscellaneous_data_structures/stack.py#L9
Stack can be used to remove recursion here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just now saw that.
Ill start making the changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
|
@nethish Are you participating through JMOC? |
|
@nethish Would you still like to work on it? Thanks. |
|
Yes. |
|
@nethish Any news on this? |
|
I have made all the changes. |
pydatastructs/trees/binary_trees.py
Outdated
| Returns | ||
| ======= | ||
| bool: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| bool: | |
| path[::-1]: bool |
pydatastructs/trees/binary_trees.py
Outdated
|
|
||
| if curr_root is None: | ||
| return curr_root | ||
| return self.tree[curr_root].key |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
key may not always be an integer. So, this conflicts with the doc string.
pydatastructs/trees/binary_trees.py
Outdated
| curr_root = self.root_idx | ||
| u, v = self.search(j), self.search(k) | ||
| if (u is None) or (v is None): | ||
| return None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A value error would have been better to let the user know that they are searching for garbage values.
|
Will merge when the tests will pass. |
Fixes #62
Added algorithms for Lowest common ancestors for binary search trees.
Added tests for LCA.