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

[ENH] DBSCAN widget #3917

Merged
merged 6 commits into from
Aug 2, 2019
Merged

[ENH] DBSCAN widget #3917

merged 6 commits into from
Aug 2, 2019

Conversation

PrimozGodec
Copy link
Contributor

@PrimozGodec PrimozGodec commented Jul 1, 2019

Description of changes

In discussion with @ajdapretnar we decided to move DBSCAN widget to core Orange. With this PR we move fixed DBSCAN widget to Orange.

Besides moving the DBSCAN widget this PR also introduce a graph that allows selection. That component is now called from OWPCA and OWDBCAN.

Note: When testing makes sure to test PCA widget too.

Includes
  • Code changes
  • Tests
  • Documentation

@codecov
Copy link

codecov bot commented Jul 1, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@37e7b02). Click here to learn what that means.
The diff coverage is 98.78%.

@@            Coverage Diff            @@
##             master    #3917   +/-   ##
=========================================
  Coverage          ?   84.76%           
=========================================
  Files             ?      373           
  Lines             ?    65063           
  Branches          ?        0           
=========================================
  Hits              ?    55148           
  Misses            ?     9915           
  Partials          ?        0

@codecov
Copy link

codecov bot commented Jul 1, 2019

Codecov Report

Merging #3917 into master will increase coverage by 0.08%.
The diff coverage is 99.32%.

@@            Coverage Diff             @@
##           master    #3917      +/-   ##
==========================================
+ Coverage   85.13%   85.21%   +0.08%     
==========================================
  Files         378      382       +4     
  Lines       67201    67590     +389     
==========================================
+ Hits        57214    57600     +386     
- Misses       9987     9990       +3

@PrimozGodec PrimozGodec changed the title [ENH] Added DBSCAN widget [WIP][ENH] Added DBSCAN widget Jul 1, 2019
@PrimozGodec PrimozGodec force-pushed the add-dbscan branch 6 times, most recently from 5f68a17 to baf3024 Compare July 11, 2019 12:56
@PrimozGodec PrimozGodec force-pushed the add-dbscan branch 4 times, most recently from 6d322ce to 58844fe Compare July 12, 2019 11:56
@PrimozGodec PrimozGodec changed the title [WIP][ENH] Added DBSCAN widget [ENH] Added DBSCAN widget Jul 12, 2019
@PrimozGodec
Copy link
Contributor Author

I think the widget is ready to merge. @BlazZupan do you have any comment regarding the widget design.

@BlazZupan
Copy link
Contributor

Works as expected on the painted data, but then may have some problems with other kinds of data sets:

  • the widget crashes on brown-selected (report below)
  • the 4-distance graph is empty on heart_disease
Traceback (most recent call last):
  File "/Users/blaz/.virtualenvs/orange/lib/python3.7/site-packages/orangecanvas/scheme/signalmanager.py", line 718, in __process_next
    self.process_queued()
  File "/Users/blaz/.virtualenvs/orange/lib/python3.7/site-packages/orangecanvas/scheme/signalmanager.py", line 476, in process_queued
    self.process_node(node_update_front[0])
  File "/Users/blaz/.virtualenvs/orange/lib/python3.7/site-packages/orangecanvas/scheme/signalmanager.py", line 523, in process_node
    self.send_to_node(node, signals_in)
  File "/Users/blaz/.virtualenvs/orange/lib/python3.7/site-packages/orangewidget/workflow/widgetsscheme.py", line 774, in send_to_node
    self.process_signals_for_widget(node, widget, signals)
  File "/Users/blaz/.virtualenvs/orange/lib/python3.7/site-packages/orangewidget/workflow/widgetsscheme.py", line 808, in process_signals_for_widget
    handler(*args)
  File "/Users/blaz/Documents/dev/orange3/Orange/widgets/unsupervised/owdbscan.py", line 187, in set_data
    self.unconditional_commit()
  File "/Users/blaz/Documents/dev/orange3/Orange/widgets/unsupervised/owdbscan.py", line 128, in commit
    self.cluster()
  File "/Users/blaz/Documents/dev/orange3/Orange/widgets/unsupervised/owdbscan.py", line 137, in cluster
    ).get_model(self.data_normalized)
  File "/Users/blaz/Documents/dev/orange3/Orange/clustering/clustering.py", line 85, in get_model
    model = self.fit_storage(data)
  File "/Users/blaz/Documents/dev/orange3/Orange/clustering/clustering.py", line 92, in fit_storage
    return self.fit(data.X)
  File "/Users/blaz/Documents/dev/orange3/Orange/clustering/clustering.py", line 95, in fit
    return self.__returns__(self.__wraps__(**self.params).fit(X))
  File "/Users/blaz/.virtualenvs/orange/lib/python3.7/site-packages/sklearn/cluster/dbscan_.py", line 351, in fit
    **self.get_params())
  File "/Users/blaz/.virtualenvs/orange/lib/python3.7/site-packages/sklearn/cluster/dbscan_.py", line 140, in dbscan
    raise ValueError("eps must be positive.")
ValueError: eps must be positive.

@PrimozGodec PrimozGodec changed the title [ENH] Added DBSCAN widget [ENH] DBSCAN widget Jul 29, 2019
@PrimozGodec
Copy link
Contributor Author

@BlazZupan it is fixed now

@ajdapretnar
Copy link
Contributor

Fails on sparse. Perhaps a quick fix with an error message to prevent it from crashing?

@PrimozGodec
Copy link
Contributor Author

@ajda it also works on bag-of-words data now.

@ajdapretnar ajdapretnar merged commit eb2c9a9 into biolab:master Aug 2, 2019
@PrimozGodec PrimozGodec deleted the add-dbscan branch August 2, 2019 12:35
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