Implementation of Density-based spatial clustering of applications with noise (DBSCAN) ML Algorithm - #11671
Implementation of Density-based spatial clustering of applications with noise (DBSCAN) ML Algorithm#11671tkgowtham wants to merge 31 commits into
Conversation
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
|
Hi, can someone please kindly review my code. All the test cases passed. If any changes/improvements needed please feel free to tell !!! |
|
@cclauss @tianyizheng02 Can you please review this PR and let me know if any changes in needed. Thanks in advance. |
|
@priya-sundaram-dev, your review, please. |
priya-sundaram-dev
left a comment
There was a problem hiding this comment.
Thanks for the contribution, @tkgowtham 🙂 I pulled the branch and ran the doctests locally with the Agg backend — all 12 pass, and the DBSCAN core/border/noise classification looks correct (each point counts itself in its ε-neighborhood, so the >= minpts core test is right).
A few suggestions, mostly small — none are hard blockers, but the type-hint one is worth fixing since the repo type-checks:
-
radiustype hint. It's annotatedradius: int, but every example passes a float (1.9,2.5). Please change it toradius: float. -
Sentinel default for
file. Using the string"None"as a sentinel (file: str = "None"+if file != "None") is fragile — a real file literally namedNonewould break it, and the attribute can actually hold either astrpath or atuple[dict, ...]. Prefer:file: str | None = None ... self.file = file if file is not None else (...)
-
isinstanceovertype() is.if type(self.file) is str:reads better asif isinstance(self.file, str):(and satisfies linters). -
Naming.
eanddict1are hard to follow —eps/radiusandneighborswould makeperform_dbscanmuch clearer for readers learning the algorithm. -
Minor / optional: standard DBSCAN uses
dist <= eps(inclusive) rather thandist < eps. Your doctests are internally consistent either way, so this is just a note. Alsoperform_dbscanis O(n²) pairwise, which is totally fine for a teaching implementation — maybe add a one-line comment saying so.
Nice, well-documented submission overall. Fix the radius: float hint and the isinstance line and I think it's in good shape. 👍
Describe your change:
Implementation of Density-based spatial clustering of applications with noise (DBSCAN) Algorithm using Python from Scratch
How it works :
Take every point and see how many other points are close to it by e (epsilon) radius distance. If that point has covered minimum of number of points (minPts), then it will be considered as Core else Noise. An Noise can also be classified as Border if it falls into the area covered by the core point.
Reference Website : https://en.wikipedia.org/wiki/DBSCAN
Reference YouTube Video : https://youtu.be/-p354tQsKrs?si=TkI9nrnAiqDHyjR-
Checklist: