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] Discretize: Manual cut points entry #4929

Merged
merged 18 commits into from
Sep 4, 2020

Conversation

ales-erjavec
Copy link
Contributor

Issue

Ref: #4914 (comment) (1)

Description of changes

Provide custom values input in Discretize widget.

Includes
  • Code changes
  • Tests
  • Documentation

@ales-erjavec ales-erjavec force-pushed the discretize-manual-point-entry branch 6 times, most recently from f101bc0 to 79696ed Compare August 3, 2020 11:31
@codecov
Copy link

codecov bot commented Aug 3, 2020

Codecov Report

Merging #4929 into master will increase coverage by 0.23%.
The diff coverage is 95.63%.

@@            Coverage Diff             @@
##           master    #4929      +/-   ##
==========================================
+ Coverage   84.24%   84.48%   +0.23%     
==========================================
  Files         283      283              
  Lines       57873    58230     +357     
==========================================
+ Hits        48757    49193     +436     
+ Misses       9116     9037      -79     

@ales-erjavec ales-erjavec force-pushed the discretize-manual-point-entry branch 4 times, most recently from d96d947 to d1e96f1 Compare August 5, 2020 10:39
@ajdapretnar
Copy link
Contributor

I really like this, but I noticed a couple of things.

  1. Manual entry works beautifully, but it would probably be nice to have a tooltip? Something like: "Separate cut-offs with a comma, use point decimal separator." Or perhaps simply: "i.e. 0.5, 1.5, 2.5". 🤷

  2. For some reason, I cannot see the tooltip on CC. Even reading it, I am not sure what it does... 🤔 I assumed it copies manual cut-offs from single variable field to the Manual field at the top. If so, it doesn't work for me.

  3. Perhaps not a part of this PR, but I have set two cut-offs, i.e. 1.5 and 2.5. There are no values that fall between 1.5 and 2.5. I then try to find them and merge with Edit Domain (merge values option), but nothing happens. Probably there's a bug somewhere. @PrimozGodec

@ales-erjavec
Copy link
Contributor Author

For some reason, I cannot see the tooltip on CC

Fixed

I assumed it copies manual cut-offs from single variable field to the Manual field at the top. If so, it doesn't work for me.

It copies the current (non-manual) cut points to the edit field and at the same time activates the Manual option.

@janezd
Copy link
Contributor

janezd commented Aug 25, 2020

Nice.

How difficult would it be to somehow indicate an invalid input? I keep forgetting commas. It would be nice to also indicate that the order is not increasing.

Are commas really necessary? Why not also accepting "1 2 3"?

In case of invalid input (e.g. in default), the output is "single_value". Any chance of renaming this to "invalid_cut_of_points"?

In Iris, I set Entropy-MDL as default, then choose sepal width and click CC. I get "2.95, 3.3499999999999996". It would be nicer to have "2.95, 3.35", which are also the values shown in the list view on the left.

Also, when I click Cc, the line edit does not refresh, but this is probably the problem with my Qt -- it does repaint if I hide and show the widget.

@janezd janezd self-assigned this Sep 3, 2020
@janezd janezd merged commit 290c2a1 into biolab:master Sep 4, 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