-
Notifications
You must be signed in to change notification settings - Fork 258
ENH - adding doc link to html repr of estimators #2036
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
Merged
rcap107
merged 25 commits into
skrub-data:main
from
rcap107:enh-add-doc-link-to-estimator
Jun 16, 2026
Merged
Changes from 17 commits
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
5fcf629
ENH - adding doc link to html repr of estimators
rcap107 baf63fb
moving the new methods
rcap107 0fde8fd
adding more
rcap107 e40ecfa
fixing applytocols
rcap107 3156828
fixing typo
rcap107 8213776
adding tests
rcap107 4a41b3f
adding a comment
rcap107 b3403c0
changelog
rcap107 ade23b8
removing unneeded setter
rcap107 fe0fc13
adding more tests for coverage
rcap107 7f774ea
Merge branch 'main' into enh-add-doc-link-to-estimator
rcap107 cb48d16
Merge remote-tracking branch 'upstream/HEAD' into enh-add-doc-link-to…
rcap107 c2eb8b5
Merge branch 'enh-add-doc-link-to-estimator' of github.com:rcap107/sk…
rcap107 1f1a828
moving changes to a single file
rcap107 79cc5e9
_
rcap107 48949d2
tests
rcap107 f1f07d9
removing unnecessary code
rcap107 ea59f93
Merge remote-tracking branch 'upstream/HEAD' into enh-add-doc-link-to…
rcap107 a1f520f
addressing comments from review
rcap107 20702bd
bringing back code block and better comment
rcap107 ce9dc05
addressing all missing files
rcap107 0fe8479
fixing relative imports
rcap107 1752ffa
renaming to estimator, fixing order, adding to paramsearch
rcap107 e4458b4
Apply suggestion from @jeromedockes
rcap107 d601e99
Merge branch 'enh-add-doc-link-to-estimator' of github.com:rcap107/sk…
rcap107 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| from sklearn.base import BaseEstimator | ||
|
|
||
|
|
||
| class BaseTransformer(BaseEstimator): | ||
|
rcap107 marked this conversation as resolved.
Outdated
|
||
| _doc_link_module = "skrub" | ||
|
|
||
| # Defining this as a property because it inherits from _HTMLDocumentationLinkMixin, | ||
| # which also defines _doc_link_template as a property, and we want to be able | ||
| # to override it. | ||
| @property | ||
| def _doc_link_template(self): | ||
| return getattr( | ||
| self, | ||
| "__doc_link_template", | ||
| "https://skrub-data.org/stable/reference/generated/" | ||
| "{estimator_module}.{estimator_name}.html", | ||
| ) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| import re | ||
|
|
||
| from sklearn.utils import estimator_html_repr | ||
|
|
||
| from skrub import ( | ||
| ApplyToCols, | ||
| Cleaner, | ||
| DropCols, | ||
| SelectCols, | ||
| StringEncoder, | ||
| TableVectorizer, | ||
| ) | ||
|
|
||
|
|
||
| def test_doc_link_apply_to_cols(): | ||
| """The wrapped transformer's doc link appears in the HTML repr of ApplyToCols.""" | ||
| html = estimator_html_repr(ApplyToCols(StringEncoder())) | ||
| links = set(re.findall(r'href="(https?://[^#"]+)"', html)) | ||
| assert ( | ||
| "https://skrub-data.org/stable/reference/generated/skrub.ApplyToCols.html" | ||
| in links | ||
| ) | ||
|
|
||
| html = estimator_html_repr(ApplyToCols(TableVectorizer())) | ||
| links = set(re.findall(r'href="(https?://[^#"]+)"', html)) | ||
| assert ( | ||
| "https://skrub-data.org/stable/reference/generated/skrub.TableVectorizer.html" | ||
| in links | ||
| ) | ||
|
|
||
|
|
||
| def test_doc_link_skrub_class_select_cols(): | ||
| """Public skrub classes get a link to skrub documentation.""" | ||
| link = SelectCols(cols=[])._get_doc_link() | ||
| assert link == ( | ||
| "https://skrub-data.org/stable/reference/generated/skrub.SelectCols.html" | ||
| ) | ||
| link = DropCols(cols=[])._get_doc_link() | ||
| assert link == ( | ||
| "https://skrub-data.org/stable/reference/generated/skrub.DropCols.html" | ||
| ) | ||
|
|
||
|
|
||
| def test_doc_link_table_vectorizer(): | ||
| """Public skrub classes get a link to skrub documentation.""" | ||
| link = TableVectorizer()._get_doc_link() | ||
| assert link == ( | ||
| "https://skrub-data.org/stable/reference/generated/skrub.TableVectorizer.html" | ||
| ) | ||
| link = Cleaner()._get_doc_link() | ||
| assert link == ( | ||
| "https://skrub-data.org/stable/reference/generated/skrub.Cleaner.html" | ||
| ) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm not sure i understood this comment, but also the scikit-learn diagram machinery is quite complicated so maybe it's not easy to explain in a short comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I added this block I had the problem that for some reason the (?) signs did not appear properly for the TableVectorizer, but now I can't replicate the problem anymore so I think it can be removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also tested sklearn 1.5, but it seems to work on the old version so not sure what happened
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nevermind, I found the problem. Without that block, the docstring for the TableVectorizer itself is not added.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without this function, the transformer inside applytocols doesn't get the doc link
with the change, it shows up properly