Skip to content
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

Allow images to define width #142

Closed
wants to merge 1 commit into from

Conversation

adamjstewart
Copy link

Fixes #140. See #140 for details.

@calebrob6

@adamjstewart
Copy link
Author

@brianjo can you review this?

@adamjstewart
Copy link
Author

@patmellon can you review this?

@ain-soph
Copy link
Contributor

As far as I know, this repo is seldom maintained...

You may propose the issue in pytorch main repo to gain more attention, if you think the modification is worth review asap.

I think this PR worth review, since the previous theme can't change image width. (but maybe not your another PR about footer modification, which is really minor.)

@brianjo
Copy link
Contributor

brianjo commented Oct 16, 2021

Adam, could you post a before and after screen so I can see what the difference is? I'll take a look on Monday and merge it after I run some tests. Thanks for the contribution!

@ain-soph
Copy link
Contributor

ain-soph commented Oct 16, 2021

@brianjo

I've tested this modification. Before this PR, all images are with 100% width, even you have written in the rst file with

.. image:: ./XXX.jpg
    :width: 60%

The generated html img tag is corrected with

<img alt="XXX.jpg" src="XXX.jpg" width="60%">

But it won't take effect because the there's style with width: 100% in the css file.

article.pytorch-article img {
  width: 100%;
}

Removing this style makes it work again. (You could see that from his changes)

@adamjstewart
Copy link
Author

As far as I know, this repo is seldom maintained...

That's unfortunate, as I think this is a great Sphinx theme and a lot of projects rely on it, including my own. I don't think I know enough about web dev to help maintain this project, but if there's anything else I can do to help let me know. I would love to see this theme install correctly without having to copy it into the root of the documentation.

@brianjo
Copy link
Contributor

brianjo commented Oct 17, 2021 via email

@ain-soph
Copy link
Contributor

ain-soph commented Oct 17, 2021

I'm currently working on my trojanzoo_sphinx_theme, which removes all pytorch-related items, and making it a pure doc theme. You can customized most things in your sphinx workspace, including logo images and permalinks (home, docs, other docs, and github_url) inside.

I've fixed many bugs I've seen, and propose PR to pytorch_sphinx_theme if possible. And I also add some minor visual changes. You may take a look if you are interested.

Example:

A demo: https://ain-soph.github.io/trojanzoo_sphinx_theme/
My python plotting package: https://ain-soph.github.io/alpsplot/

@brianjo
Copy link
Contributor

brianjo commented Oct 17, 2021 via email

@adamjstewart
Copy link
Author

This looks great! I probably won't switch to this until the rest of the PyTorch community switches though. Trying to maintain some semblance of uniformity in docs. Is the PyTorch community willing to switch to this? It seems pointless to have two repos if we have to submit bug fixes to both.

@ain-soph
Copy link
Contributor

This looks great! I probably won't switch to this until the rest of the PyTorch community switches though. Trying to maintain some semblance of uniformity in docs. Is the PyTorch community willing to switch to this? It seems pointless to have two repos if we have to submit bug fixes to both.

I'm not expecting to this though. Currently pytorch_sphinx_theme has a lot of items specially designed for pytorch, including additional social links and other links in layout.html.
A possible solution would be to put the special layout.html in /docs of pytorch and tutorial repos (which they already have for some additional minor modification), and make pytorch_sphinx_theme as a generic theme.

@adamjstewart
Copy link
Author

A possible solution would be to put the special layout.html in /docs of pytorch and tutorial repos (which they already have for some additional minor modification), and make pytorch_sphinx_theme as a generic theme.

Yes, this is exactly what I'm hoping for. Since my project is also a PyTorch domain library, I would like to keep my docs consistent with the other ones. I think this also represents a best case for the PyTorch devs too since they won't have to maintain this repo anymore.

@adamjstewart
Copy link
Author

adamjstewart commented Dec 8, 2021

Closing this as it seems that this repo is not maintained. We're starting to seriously consider the trojanzoo_sphinx_theme since it's actually maintained.

@adamjstewart adamjstewart deleted the fixes/width branch December 8, 2021 22:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to define width of image
4 participants