Skip to content

[ENH] Bar Plot: New widget#4923

Merged
janezd merged 2 commits intobiolab:masterfrom
VesnaT:bar_plot
Oct 2, 2020
Merged

[ENH] Bar Plot: New widget#4923
janezd merged 2 commits intobiolab:masterfrom
VesnaT:bar_plot

Conversation

@VesnaT
Copy link
Contributor

@VesnaT VesnaT commented Jul 27, 2020

Issue

Add Bar Plot widget.

Description of changes

I'll add the docs when the UI is approved.

Includes
  • Code changes
  • Tests
  • Documentation

@VesnaT VesnaT force-pushed the bar_plot branch 7 times, most recently from 822c80f to b4d8d08 Compare July 27, 2020 12:01
Copy link
Contributor

@janezd janezd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grouping is more like sorting, isn't it? I think group either have to be clearly separated, or, better still, we could call it sorting and include numeric attributes. In this case I'd even add an option "by value" above "none", and it would be the default.

Annotations will almost never be visible, I suppose. What should we do about them? @irgolic?

return str.replace("<=", "&#8804;").replace(">=", "&#8805;").\
def to_html(str_):
return str_.replace("<=", "&#8804;").replace(">=", "&#8805;").\
replace("<", "&#60;").replace(">", "&#62;").replace("=\\=", "&#8800;")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about return escape(str_, entities={"<=": "&#8804;", ">=": "&#8805;", "=\\=": "&#8800;"})?

By the way, for long sequences of replacements I sometimes prefer

reduce(lambda s, t:s.replace(*t), (
    ("<=", "&#8804;"), 
    (">=", "&#8805;"),
    ("<", "&#60;"),
    (">", "&#62;"),
    ("=\\=", "&#8800;")
    )), s)

But this is a matter of taste, and escape fits better here anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above..

@VesnaT
Copy link
Contributor Author

VesnaT commented Jul 30, 2020

Annotations will almost never be visible, I suppose.

Why?

@janezd
Copy link
Contributor

janezd commented Jul 30, 2020

Annotations will almost never be visible, I suppose.

Why?

I meant, they will almost always overlap.

@VesnaT
Copy link
Contributor Author

VesnaT commented Jul 30, 2020

Almost never. They are displayed vertically and one can set the font size.
I added a None option, though.

@VesnaT VesnaT force-pushed the bar_plot branch 2 times, most recently from db42742 to c212a4d Compare July 30, 2020 12:39
@janezd
Copy link
Contributor

janezd commented Jul 31, 2020

Even with 100 instances and 10 px labels, this already spreads to > 1000 pixels. But maybe this widget won't be used liked that -- I confess I don't see it's actual use (but it's just me).

Adding None suffices for me. :)

@lanzagar
Copy link
Contributor

Even with 100 instances and 10 px labels, this already spreads to > 1000 pixels. But maybe this widget won't be used liked that -- I confess I don't see it's actual use (but it's just me).

Adding None suffices for me. :)

We were talking about problems when trying to visualize too many bars and how to prevent that. The first idea was to limit it to ~50 rows of data. Then after seeing that the actual bar plot works quite well even for 150 instances, we agreed to limit it to 200 instead. Obviously annotations become a problem in these cases. Since it is now easy to disable them and even to adjust the font sizes I think we can leave the limit at 200, but I would be fine with reducing it to 100 as well.

The main motivation for this widget was to show 3-20 values with a bar chart since that was a common request by people using Orange, who where then forced to use a different tool to produce bar charts of Orange's results. For that scope the annotations should not be a problem and are probably an important feature to have.

@janezd
Copy link
Contributor

janezd commented Jul 31, 2020

Alright then. As I said, I didn't know how this was intended to be used.

@codecov
Copy link

codecov bot commented Sep 18, 2020

Codecov Report

Merging #4923 into master will decrease coverage by 0.04%.
The diff coverage is 79.54%.

@@            Coverage Diff             @@
##           master    #4923      +/-   ##
==========================================
- Coverage   84.66%   84.61%   -0.05%     
==========================================
  Files         284      285       +1     
  Lines       58942    59417     +475     
==========================================
+ Hits        49902    50276     +374     
- Misses       9040     9141     +101     

@janezd janezd marked this pull request as draft September 24, 2020 16:01
@janezd janezd marked this pull request as ready for review October 2, 2020 07:05
@janezd janezd merged commit 2c32d99 into biolab:master Oct 2, 2020
@janezd janezd mentioned this pull request Oct 2, 2020
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