-
Notifications
You must be signed in to change notification settings - Fork 186
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
Example data fits Sphinx Gallery's image dimensions #1246
Conversation
👋 Hey, looks like you've updated some demos! 🐘 Don't forget to update the Please hide this comment once the field(s) are updated. Thanks! |
Thank you for opening this pull request. You can find the built site at this link. Deployment Info:
Note: It may take several minutes for updates to this pull request to be reflected on the deployed site. |
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.
Thanks @doctorperceptron ! Just one small question. If the settings on sphix negate specifying figsize
than your fix looks good to me.
for i in range(5): | ||
axes[i].matshow(X_train[2*i]) | ||
axes[i].axis('off') | ||
fig, axes = plt.subplots(nrows=2, ncols=3, layout="constrained") |
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.
Would specifyingfigsize
inside of subplots help or would that be superseded by the display settings in sphinx? It looks better but seems disproportionately big
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.
It might! Let's find out...
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.
Hmm small problem, because we use matplotlib inline
the figure gets cropped by Jupyter and makes the actual size unpredictable. I think this is the best we can do without disabling matplotlib inline (currently set globally for all demos).
https://matplotlib.org/stable/gallery/subplots_axes_and_figures/figure_size_units.html#id1
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.
Ok - thanks for checking it out. In that case, the fix is fine with me.
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.
lgtm
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.
LGTM!
Just a note, this new figure layout introduces a 6th figure. Not sure if that's an issue or not given the context of this demo. cc @ikurecic
These figures are a representative sample of the data. Afaik the initial choice of 5 was arbitrary, so changing it to 6 shouldn't material affect the demo. |
Title:
Adapt example data in demo to fit Sphinx Gallery's image dimensions
Summary:
Sphinx Gallery renders single images at 640px by 480px. Adjusting the layout of the example data to better fit these dimensions and reduce white space.
Relevant references:
https://app.shortcut.com/xanaduai/story/76532
Before:
After: