ENH Adding a column to link each parameter docstring row in params table display #31564

DeaMariaLeon · 2025-06-17T08:44:59Z

Reference Issues/PRs

Adds a column with a link to each parameter docstring in the params table (in the HTML display).
This was discussed while working on the PR that added the table.

"Text fragments" code shared by @ogrisel here :
#30763 (comment)

What does this implement/fix? Explain your changes.

Added a column to the table with the new link
Modified ParamsDict to include the estimator_type. After several tries it was the only way I found at the moment.

Any other comments?

@glemaitre

github-actions · 2025-06-17T08:45:58Z

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

_{Generated for commit: b55bfc6. Link to the linter CI: here}

DeaMariaLeon · 2025-06-18T10:42:10Z

~~Example here~~

TODO:

Address Olivier's comment (during a call). He mentioned that the ReprHTMLMixin could be set-up in a way that we only add documentation link of estimators that have similar HTML structure.
Test for broken links
The "?" link is too small, but it's the same size of the text. ~~latest doc built~~ - It seems that having the same text for each hyperlink is different than the recommended "best practices".

DeaMariaLeon · 2025-07-07T07:17:04Z

sklearn/utils/_repr_html/params.py

@@ -23,6 +76,31 @@ def _read_params(name, value, non_default_params):
    return {"param_type": param_type, "param_name": name, "param_value": cleaned_value}



It seems that r.maxstringis not working.
Noticed while working on adding methods table.

Maybe open a dedicated issue with a reproducer?

jeremiedbb

Thanks for the PR @DeaMariaLeon.

I tested on the example you linked and it worked fine except for the C parameter of LogisticRegression. Maybe the text fragment requires more than 1 character ?

Besides that, here are some comments

sklearn/base.py

sklearn/utils/_repr_html/params.py

DeaMariaLeon · 2025-07-22T08:42:15Z

tested on the example you linked and it worked fine except for the C parameter of LogisticRegression. Maybe the text fragment requires more than 1 character ?

Thanks for the feedback! - the link to C that you mention works for me locally. But the link to my example doesn't work (the whole example, not the parameter link). Maybe on the next doc build I can see it.. ?
I addressed some of the comments you left. I'll work on the rest.

sklearn/utils/_repr_html/params.css

DeaMariaLeon · 2025-08-25T10:07:19Z

Docs have been built if someone wants to try again.

glemaitre · 2025-08-25T12:00:41Z

The rendering looks good to me now. @jeremiedbb you can have another look.

I'll go through the tests once more.

GaelVaroquaux · 2025-08-25T13:14:49Z

After changes requested by Guillaume example

This is soooo cool!!

GaelVaroquaux · 2025-08-25T13:20:33Z

One comment: can we (should we), add "display: block;" to "a.param-doc-link, a.param-doc-link:link, a.param-doc-link:visited" so that its hovering on the cell rather than on the text that triggers the tooltip?
I find it more pleasant from an interaction standpoint.

It probably means that we should turn off the dashed underline on the text, because with "display: block" extends the underline too far out.

DeaMariaLeon · 2025-08-25T13:30:08Z

~~If we remove the dashed underline, I wonder how we should show that there is a link to the actual documentation on the website. "Click to see the docs" at the end of the tooltip doc maybe?~~

edit: We could keep a dashed line...
@GaelVaroquaux

GaelVaroquaux · 2025-08-25T13:55:24Z

edit: We could keep a dashed line...

Very nice!!

glemaitre · 2025-08-25T14:01:36Z

I'll like to keep the underline to alert users that there is something special about the this section of the diagram. I would still expect the text to have the hyperlink towards the documentation. Regarding text vs. cell, I don't have a strong opinion (maybe a slight one for the text since I find it easier to disable the tooltip by moving away from the string but it is rather subjective so using the cell is fine as well).

ogrisel · 2025-08-25T15:14:48Z

I'm not sure that linking to the full doc page is necessary then but why not if you want to have the rst properly rendered.

+1 for keeping the link. Sometimes the rendering can be helpful (e.g. math). Furthermore, when browsing from a mobile device without a mouse, there is no way to get the tooltip so better keep a "click to navigate to doc" interaction in addition to the tooltip.

DeaMariaLeon · 2025-08-26T07:11:47Z

The docs have been built. Gaël's feedback was added.
Example here

GaelVaroquaux · 2025-08-26T07:20:49Z

I haven't reviewed the code, but from a user perspective, I think that it looks absolutely great, and it's definitely mergeable for me!! Thanks!

glemaitre

OK here is a pass on the code more in details. I propose some slight changes.

sklearn/base.py

sklearn/utils/_repr_html/estimator.js

sklearn/utils/_repr_html/params.py

sklearn/utils/_repr_html/tests/test_params.py

DeaMariaLeon · 2025-08-27T09:22:36Z

@glemaitre I think I addressed all your comments.

DeaMariaLeon added 3 commits June 16, 2025 20:08

wip

503dc3c

wip

03ccaf7

wip

8376ce4

github-actions bot added the module:utils label Jun 17, 2025

DeaMariaLeon added 2 commits June 17, 2025 11:25

fix one test

1dd81b9

fixed regex

f580197

glemaitre self-requested a review June 17, 2025 21:20

DeaMariaLeon added 4 commits June 18, 2025 10:23

adding ? symbol to link

69edbe1

added hover

da427f0

better message

b067542

[doc build] Added docstring

32ff382

DeaMariaLeon added 10 commits June 18, 2025 13:47

fixed steps link in pipeline

0ae3124

[doc build] trigger doc build

5e8bd5a

[doc build] fixed link colors

a4c98e6

Merge remote-tracking branch 'upstream/main' into doclink

0804529

added test

c7ee9da

wip

5cfc016

fixed link when not found

19549c3

wip

a8bb530

[doc build] trigger doc build

316cd14

[doc build] try to fix space

d5381ac

DeaMariaLeon marked this pull request as ready for review June 20, 2025 12:25

DeaMariaLeon changed the title ~~WIP: ENH Adding a column to link each parameter docstring row in params table display~~ ENH Adding a column to link each parameter docstring row in params table display Jun 20, 2025

DeaMariaLeon commented Jul 7, 2025

View reviewed changes

jeremiedbb reviewed Jul 21, 2025

View reviewed changes

Partially addressing feedback

c7a0aa8

Import at the top

074a388

DeaMariaLeon added 2 commits August 22, 2025 17:39

[doc build] max-height change

c84354f

dashed line

6321248

glemaitre reviewed Aug 25, 2025

View reviewed changes

sklearn/utils/_repr_html/params.css Outdated Show resolved Hide resolved

[doc build] fix dashed line & test

993f732

glemaitre self-requested a review August 25, 2025 08:32

[doc build] removed extra space

8f5c715

glemaitre reviewed Aug 25, 2025

View reviewed changes

sklearn/utils/_repr_html/params.css Show resolved Hide resolved

DeaMariaLeon added 2 commits August 25, 2025 10:58

[doc build] visited link style

7e18ae9

[doc build] Added comments to css

f62c5d0

[doc build] Hovering on cell

81c7fcc

glemaitre reviewed Aug 26, 2025

View reviewed changes

DeaMariaLeon added 8 commits August 26, 2025 15:20

doc_link at the end

654d2de

shorter line

dc86313

modified test, make keyword only, add default values, modified for loop

1ba6998

added regex to test_params

1edf99b

Remove __doc__ from tests in test_params

0e9f526

Modified base

050568c

Merge remote-tracking branch 'upstream/main' into doclink

5769213

[doc build] forgot doc build

b55bfc6

		@@ -23,6 +76,31 @@ def _read_params(name, value, non_default_params):
		return {"param_type": param_type, "param_name": name, "param_value": cleaned_value}

Uh oh!

ENH Adding a column to link each parameter docstring row in params table display #31564

Are you sure you want to change the base?

ENH Adding a column to link each parameter docstring row in params table display #31564

Conversation

DeaMariaLeon commented Jun 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Any other comments?

Uh oh!

github-actions bot commented Jun 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✔️ Linting Passed

Uh oh!

DeaMariaLeon commented Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DeaMariaLeon Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ogrisel Jul 29, 2025

Choose a reason for hiding this comment

Uh oh!

jeremiedbb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DeaMariaLeon commented Jul 22, 2025

Uh oh!

Uh oh!

Uh oh!

DeaMariaLeon commented Aug 25, 2025

Uh oh!

glemaitre commented Aug 25, 2025

Uh oh!

GaelVaroquaux commented Aug 25, 2025

Uh oh!

GaelVaroquaux commented Aug 25, 2025

Uh oh!

DeaMariaLeon commented Aug 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GaelVaroquaux commented Aug 25, 2025

Uh oh!

glemaitre commented Aug 25, 2025

Uh oh!

ogrisel commented Aug 25, 2025

Uh oh!

DeaMariaLeon commented Aug 26, 2025

Uh oh!

GaelVaroquaux commented Aug 26, 2025

Uh oh!

glemaitre left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DeaMariaLeon commented Aug 27, 2025

Uh oh!

Uh oh!

DeaMariaLeon commented Jun 17, 2025 •

edited

Loading

github-actions bot commented Jun 17, 2025 •

edited

Loading

DeaMariaLeon commented Jun 18, 2025 •

edited

Loading

DeaMariaLeon Jul 7, 2025 •

edited

Loading

DeaMariaLeon commented Aug 25, 2025 •

edited

Loading