Skip to content

Faster svg viewbox computation in tablereport#1138

Merged
jeromedockes merged 3 commits intoskrub-data:mainfrom
jeromedockes:faster-svg-bbox-adjustments
Nov 15, 2024
Merged

Faster svg viewbox computation in tablereport#1138
jeromedockes merged 3 commits intoskrub-data:mainfrom
jeromedockes:faster-svg-bbox-adjustments

Conversation

@jeromedockes
Copy link
Member

The way the computation of the correct viewbox for the plots is done currently is slow when there are many columns.
This makes all the bounding box computation in one go with no changes to the document in between to avoid unnecessary recomputations of the styles and layout

also as it adds a few functions to the report code, now is a good time to put it in a module to avoid polluting the global namespace so this PR adds that too

Copy link
Member

@GaelVaroquaux GaelVaroquaux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but I think that it would be useful to add a short changelog entry: it's user-visible enough

@jeromedockes
Copy link
Member Author

thanks @GaelVaroquaux . the computation of bounding boxes in the browser has not been released yet so I did not create a changelog entry but I added this pr's number to the entry about improving the svg text

@jeromedockes jeromedockes marked this pull request as draft November 15, 2024 09:58
@jeromedockes
Copy link
Member Author

I marked it as draft because @Vincent-Maladiere agreed to check it works in a different environment so we should wait for that info before merging

Copy link
Member

@Vincent-Maladiere Vincent-Maladiere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM after testing on my machine

@jeromedockes
Copy link
Member Author

thanks a lot!

@jeromedockes jeromedockes marked this pull request as ready for review November 15, 2024 10:12
@jeromedockes jeromedockes merged commit 8808526 into skrub-data:main Nov 15, 2024
@jeromedockes jeromedockes deleted the faster-svg-bbox-adjustments branch November 15, 2024 10:12
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