DOC Update _hdbscan/_linkage.pyx with new inline comments
#25656
Conversation
There was a problem hiding this comment.
I'm always concerned with having inline comments that appear between every line of code. They often describe "what" the code is doing and not "why". In these cases, I prefer to improve the code itself, either by simplifying the code or having better variable names.
| min_reachability = np.full(n_samples, fill_value=np.infty, dtype=np.float64) | ||
| for i in range(0, n_samples - 1): | ||
| # Sub-select nodes not-yet in the tree |
There was a problem hiding this comment.
Can this be communicate with a better variable name for label_filter?
| # Compute the nodes' current min-reachability scores | ||
| left = min_reachability[label_filter] | ||
| # Compute the nodes' mutual-reachability to current node | ||
| right = mutual_reachability[current_node][current_labels] |
There was a problem hiding this comment.
left and right are not computing anything. Is there a better name for left and right such that the comment is not required?
| # Update min-reachability, given the new mutual-reachability of all | ||
| # nodes from the current node. |
There was a problem hiding this comment.
Also would a better name for left and right remove the need for this comment?
There was a problem hiding this comment.
LGTM once @thomasjpfan's comments are addressed. Thank you, @Micky774.
Moreover, can you add a TODO to state that mst_from_data_matrix can be backed later by a PairwiseDistancesReduction back-end?
FYI, I have added an item to #22587. |
Reference Issues/PRs
Follow-up to #24857
What does this implement/fix? Explain your changes.
Improves in-line comments to address algorithm steps directly
Any other comments?