You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
makeclusters() in DataClusters class undefined #64
This function only looks empty, it is actually foundational to the existing implementation. I'll summarize the current behavior and how it could be changed, if you wish to do so.
DataClusters is an iterable because it defines the __iter__ method. Python defines that the next() method returns the next member of an iterable. So when makeclusters() is run it is actually calling next() many times, and that is where the bulk of the work is done. That is why next() is never called explicitly -- it is done implicitly in makeclusters(). So the iteration is not over the clusters that have been found, but the process of clustering. I comment in the source that "This behavior could cause confusion and should perhaps be altered." Maybe that time is now?
If you wanted to change this behavior to make the clustering process more explicit (and thus easier to read/maintain), here is a possible approach.
Remove __iter__
Rename next() to cluster_next_point() or similar
Rewrite makeclusters() into a loop that updates current_idx and calls cluster_next_point()
Revise cluster_next_point() so it no longer updates current_idx and (probably) no longer returns self.
Write some good tests on non-trivial data to make sure nothing has broken.
Activity
sbillinge commentedon Aug 14, 2024
Please can you add more information on this issue so another person could read it and know what to do? Thx
stevenhua0320 commentedon Aug 14, 2024
This function is a empty function and needs refactor. It needs to based on
DataClustersobject's data points and resolution to make clusters.Ainamacar commentedon Aug 14, 2024
This function only looks empty, it is actually foundational to the existing implementation. I'll summarize the current behavior and how it could be changed, if you wish to do so.
DataClusters is an iterable because it defines the
__iter__method. Python defines that thenext()method returns the next member of an iterable. So whenmakeclusters()is run it is actually callingnext()many times, and that is where the bulk of the work is done. That is whynext()is never called explicitly -- it is done implicitly inmakeclusters(). So the iteration is not over the clusters that have been found, but the process of clustering. I comment in the source that "This behavior could cause confusion and should perhaps be altered." Maybe that time is now?If you wanted to change this behavior to make the clustering process more explicit (and thus easier to read/maintain), here is a possible approach.
__iter__next()tocluster_next_point()or similarmakeclusters()into a loop that updatescurrent_idxand callscluster_next_point()cluster_next_point()so it no longer updatescurrent_idxand (probably) no longer returns self.sbillinge commentedon Aug 14, 2024
Thanks for the feeback @Ainamacar .
Ainamacar commentedon Aug 16, 2024
Edit: I moved a lengthy comment to PR #73, where it was more appropriate.