MNT remove HF-specific overflow injection from model plot#517
Merged
adrinjalali merged 6 commits intoMay 26, 2026
Conversation
Newer sklearn versions (1.9+) emit `sk-top-container` both as a CSS
class definition (.sk-top-container.sk-global { ... }) and as an HTML
class attribute. The previous naive string replace only ran when the
token appeared exactly once and would have corrupted the CSS otherwise.
Use a regex that targets the class attribute on the top <div> so the
inline `style="overflow: auto;"` is added correctly regardless of
how many times the token appears in the HTML output.
The inline `style="overflow: auto;"` was added to work around how sklearn's HTML repr renders inside the HuggingFace Hub models page. skops no longer specifically targets HF Hub (see skops-dev#462 and skops-dev#512), so this HF-specific HTML manipulation is no longer needed. This also resolves the breakage with sklearn 1.9+ where the previous naive string replace would have corrupted the new CSS that now also mentions `sk-top-container`.
adrinjalali
changed the title
Also adds narwhals as a transitive dep of sklearn nightly, which was missing from the v6 lock and would have broken the sklearn-nightly CI once the new nightly wheels start depending on narwhals.
adrinjalali
commented
May 26, 2026
|
|
||
| """ | ||
| model_plot_div = re.sub(r"\n\s+", "", str(estimator_html_repr(model))) | ||
| if model_plot_div.count("sk-top-container") == 1: |
Member
Author
There was a problem hiding this comment.
these were for HF specific overflow issue which is not used anymore.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
The sklearn nightly tests started failing on PR #516 because newer sklearn (1.9+) emits
sk-top-containertwice in the HTML repr — once as a CSS selector (.sk-top-container.sk-global { ... }) and once as an HTML class attribute. A naive string replace would have corrupted the new CSS.Looking at why the injection existed in the first place: it was specifically to work around how sklearn's HTML repr renders inside the HuggingFace Hub models page. Since skops no longer targets HF Hub (see #462 and #512), this HF-specific HTML manipulation is no longer needed.
style=\"overflow: auto;\"injection from_add_model_plot.test_no_overflowtest.Test plan
pixi run -e ci-sklearn18 pytest skops/card/tests/test_card.py(140 passed)