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

Minor bug fix for flash making #137

Merged
merged 2 commits into from
Aug 31, 2024
Merged

Minor bug fix for flash making #137

merged 2 commits into from
Aug 31, 2024

Conversation

YifanC
Copy link
Collaborator

@YifanC YifanC commented Aug 21, 2024

No description provided.

@YifanC YifanC requested review from diaza and liviocali August 21, 2024 01:41
@liviocali
Copy link
Member

DBSCAN1D was used as a more efficient implementation of sklearn dbscan but I am ok with moving back to the sklearn version

@YifanC
Copy link
Collaborator Author

YifanC commented Aug 21, 2024

@liviocali Do you have quantified number for performance of how much more efficient is DBSCAN1D is compared to sklearn? From my two-file test, using sklearn is acceptable. I would say it's a good practice to avoid adding new dependencies unless it's a new tool or it provides significant improvement.

@YifanC YifanC requested a review from mjkramer August 23, 2024 21:43
@mjkramer mjkramer merged commit 34d2dd3 into develop Aug 31, 2024
2 checks passed
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.

3 participants