NotificationsYou must be signed in to change notification settings
Fork7.9k
Star21.4k

Decrease the thumbnail resolution in sphinx-gallery for performences#17275

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Closed

sciunto wants to merge1 commit intomatplotlib:masterfromsciunto:sphinx

Closed

Decrease the thumbnail resolution in sphinx-gallery for performences#17275

sciunto wants to merge1 commit intomatplotlib:masterfromsciunto:sphinx

Conversation

Copy link

PR Summary

This PR brings a reduction of ~2x on the thumbnail size in the gallery, which improves significantly the loading time.

Same PR as scikit image:scikit-image/scikit-image#4613
Submitted after@tacaswell 's suggestion.

PR Checklist

Has Pytest style unit tests
Code isFlake 8 compliant
New features are documented, with examples if plot related
Documentation is sphinx and numpydoc compliant
Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

Decrease the thumbnail resolution in sphinx-gallery for performences

e35f0ed

jklymak reviewed

Apr 30, 2020

View reviewed changes

doc/conf.py

		# Default thumbnail size (400, 280)
		# Default CSS rescales (160, 112)
		# Size is decreased to reduce webpage loading time
		'thumbnail_size': (280, 196),

Copy link

Member

Choose a reason for hiding this comment

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

Why a factor of 1.75 instead of 2? Not sure of the practical benefit of this since our hosting provider compresses the pngs for us, but it looks fine even at 1.75...

Copy link

Author

Choose a reason for hiding this comment

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

Visually, it would be too much.

Copy link

Member

Choose a reason for hiding this comment

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

Not sure I follow... There arelots of high-DPI/retina displays in the wild, so CSS 160 is really 320 pixels and hence a factor of 2 makes sense.

Copy link

Member

tacaswell commentedMay 1, 2020

This pulls ~7MB, the current docs pulls about 9MB from cloudflare (who does some on the fly compression for us). I can't imagine starting with slightly smaller images to begin with wouldn't help (even if our net gain actual gain is less than you would expect).