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] Heatmap: Split columns, Column annotations #4703

Merged
merged 16 commits into from
May 29, 2020

Conversation

ales-erjavec
Copy link
Contributor

@ales-erjavec ales-erjavec commented Apr 25, 2020

Issue

Implements gh-4661

Builds on: gh-4686 which should be merged before this. (was merged)

Description of changes

Implement split by column, and column color annotations.

Includes
  • Code changes
  • Tests
  • Documentation

@codecov
Copy link

codecov bot commented May 8, 2020

Codecov Report

Merging #4703 into master will increase coverage by 0.00%.
The diff coverage is 91.60%.

@@           Coverage Diff            @@
##           master    #4703    +/-   ##
========================================
  Coverage   84.01%   84.01%            
========================================
  Files         281      277     -4     
  Lines       56899    56391   -508     
========================================
- Hits        47801    47377   -424     
+ Misses       9098     9014    -84     

@ales-erjavec ales-erjavec force-pushed the heatmap-split-columns branch 3 times, most recently from 7b97866 to de0f866 Compare May 11, 2020 10:43
@ales-erjavec ales-erjavec changed the title [WIP][ENH] Heatmap: Split columns, Column annotations [ENH] Heatmap: Split columns, Column annotations May 11, 2020
@Hrovatin
Copy link
Contributor

After this PR Louvain clustering stops working.


Traceback (most recent call last):
  File "/home/karin/Documents/git/orange3venv/lib/python3.6/site-packages/orangecanvas/scheme/signalmanager.py", line 936, in __process_next
    if self.__process_next_helper(use_max_active=True):
  File "/home/karin/Documents/git/orange3venv/lib/python3.6/site-packages/orangecanvas/scheme/signalmanager.py", line 974, in __process_next_helper
    self.process_node(selected_node)
  File "/home/karin/Documents/git/orange3venv/lib/python3.6/site-packages/orangecanvas/scheme/signalmanager.py", line 605, in process_node
    self.send_to_node(node, signals_in)
  File "/home/karin/Documents/git/orange-widget-base/orangewidget/workflow/widgetsscheme.py", line 792, in send_to_node
    self.process_signals_for_widget(node, widget, signals)
  File "/home/karin/Documents/git/orange-widget-base/orangewidget/workflow/widgetsscheme.py", line 826, in process_signals_for_widget
    handler(*args)
  File "/home/karin/Documents/git/orange3/Orange/widgets/unsupervised/owlouvainclustering.py", line 439, in set_data
    self.commit()
  File "/home/karin/Documents/git/orange3/Orange/widgets/unsupervised/owlouvainclustering.py", line 255, in commit
    k_neighbors, metric = self.k_neighbors, METRICS[self.metric_idx][1]
IndexError: list index out of range

@Hrovatin
Copy link
Contributor

Hrovatin commented May 15, 2020

Except for the above, the splitting and annotation work. There is a problem with overlapping legend values (see image). The Annotation titles do not have the same font size (see image) - this seems to depend on number of rows/clusters - sometimes one annotation title is larger and in other cases the other.
Screenshot from 2020-05-15 16-08-08

One of the annotation legends overlaps the heatmap.
Screenshot from 2020-05-15 16-25-01

Not directly related to this PR:
When values are present in the domain but not the data they are still included in the Split by, so the annotations can look like they are shifted. Example for row splitting variable (e.g. missing C1, C20, C5,..). Could it be made more clear that some values are shown but do not have any data - maybe colored in gray or a warning message?
Screenshot from 2020-05-15 16-11-51

@ales-erjavec
Copy link
Contributor Author

After this PR Louvain clustering stops working.

I do not think that could be related to this.

One of the annotation legends overlaps the heatmap.

Fixed.

The Annotation titles do not have the same font size

Fixed.

There is a problem with overlapping legend values

Fixed.

@Hrovatin
Copy link
Contributor

The previously pointed out bugs seem to be gone.

The numeric/continuous variable column legend is missing a title (categorical legends for columns and rows have them). It is also positioned higher than the first row of the categorical row annotation legend, although this is only a minor problem (solely for aesthetics).

Sometimes (e.g. I selected Zoom to fit) the annotation titles still get small (Time, Cluster) and row group titles overlap (see image below). Interestingly, this did not happen on the transposed version.
Screenshot from 2020-05-28 12-12-41

(Unrelated to this PR: Sometimes Zoom to fit does not work immediately, so I need to first select Zoom out a couple of times before selecting Zoom to fit to make it work. )

I still get the Louvain clustering error - I do not get it on a master from a few days ago. It is probably not relevant for this PR.

@ales-erjavec
Copy link
Contributor Author

I have added a title for continuous gradient legend and fixed annotation column titles size (hopefully for good).

@Hrovatin
Copy link
Contributor

The fixes work as expected.

@janezd janezd merged commit e35913b into biolab:master May 29, 2020
@ales-erjavec ales-erjavec deleted the heatmap-split-columns branch August 5, 2020 11:43
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