Skip to content

Conversation

@janezd
Copy link
Contributor

@janezd janezd commented Dec 31, 2025

Issue

Resolves #6984.

Description of changes

I adopted a term "General preset" and consistently use it in both widges.

The Discretize widget had very large spacing between radio buttons; the height was made uniform by adjusting all heights to the largest height. Now the height of smaller items is adjusted to the mean between minimal and maximal. This looks better, imho.

Includes
  • Code changes

@janezd janezd force-pushed the unifiy-discretize-continuize branch from 075b422 to 58064b2 Compare December 31, 2025 14:19
@codecov
Copy link

codecov bot commented Dec 31, 2025

Codecov Report

❌ Patch coverage is 94.11765% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 89.04%. Comparing base (246474b) to head (0cbe30b).
⚠️ Report is 4 commits behind head on master.

❌ Your patch check has failed because the patch coverage (94.11%) is below the target coverage (95.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7232      +/-   ##
==========================================
+ Coverage   89.03%   89.04%   +0.01%     
==========================================
  Files         335      335              
  Lines       74243    74238       -5     
==========================================
+ Hits        66099    66103       +4     
+ Misses       8144     8135       -9     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ajdapretnar
Copy link
Contributor

I like the choice of "General preset". However, if I might nitpick...
Discretize shows, i.e. age (as preset). Continuize doesn't show anything, unless in some weird (irreproducible) instance when it shows gender: preset upon hover. Would it be possible to unify this, too? So both show the same thing. I propose age: preset (semicolon).
Oh, also, move Use general preset in Discretize to index 0, as is in Continuize.
But again, it's nitpicking, so happy to merge as is (unless we can reproduce the weird on hover issue).

@janezd janezd force-pushed the unifiy-discretize-continuize branch from b307261 to 585932d Compare January 9, 2026 15:56
@janezd
Copy link
Contributor Author

janezd commented Jan 9, 2026

I followed your suggestions. I actually noticed all of these when making the PR. :)

  • The idea behind showing individual settings on tooltip (in Continuize) was to reduce clutter and I thought to add it to Discretize, too, but was too lazy. Now I see it's unreliable and not really needed, so settings are now always shown in both widgets.
  • I danced around colons and parentheses a lot. Now I did as you suggested.
  • I was scared to change the order of radio buttons, but now I see widget is actually implemented well, and is resilient to changing the order.

Please squash the commits when merging.

@janezd janezd force-pushed the unifiy-discretize-continuize branch 3 times, most recently from 1dc6bcf to 273e582 Compare January 9, 2026 19:47
@janezd janezd force-pushed the unifiy-discretize-continuize branch from 273e582 to 0cbe30b Compare January 9, 2026 20:56
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.

Unify Discretize and Continuize

2 participants