Add sphinx-tabs + use it for OS-specific "py[thon] -m pip" #8589
Conversation
docs/html/installing.rst
Outdated
|
|
||
|
|
||
| On Windows [4]_:: | ||
| On Windows, Linux or macOS:: |
There was a problem hiding this comment.
Since this doesn't categorize anything (and implies that on other OSes the method would be different), I suggest replacing this by a filler, e.g. in a shell or something like that.
There was a problem hiding this comment.
I changed it to "In a shell". Thank you.
There was a problem hiding this comment.
Do we anywhere make the point that you don't always invoke Python using the python command? I feel as though we should clarify this somewhere if we're going to make python -m pip be the canonical way we say "run pip"...
(This is arguably off-topic for this PR, and honestly, arguably not even pip's problem to address, but I feel that without some such clarification, this change is attacking a symptom, rather than the cause, of the problem people have invoking pip).
There was a problem hiding this comment.
Adding some context on why we should use python -m pip should help. Should I create a new page or add it to an existing page? We can reference Brett Cannon's article https://snarky.ca/why-you-should-use-python-m-pip/ there.
There was a problem hiding this comment.
Sorry, I wasn't clear. I'm fine with the -m pip bit. The problem is that on Windows, python probably isn't on PATH, you should use py. On Unix, you should often use python3 rather than python. Unless you're in a virtual environment. Etc.
Using -m pip means that you don't need to think about how the pip command relates to whatever way you run Python. But it leaves you still needing to know how to invoke Python. We're using python as a shortcut for "whatever command you use to run Python" and I don't see anything explaining that (specifically "we don't necessarily mean you should type python literally")...
There was a problem hiding this comment.
I think there's also a need to mention that on Windows Python version is specified via py -<major>.<minor> instead of python<major>.<minor>.
There was a problem hiding this comment.
@pfmoore - Please let me know how to proceed? Does this change need further discussion before I proceed to making more changes?
There was a problem hiding this comment.
I'd like to know what other @pypa/pip-committers think.
There was a problem hiding this comment.
We can have a box on top of each page to tell the user they should use pythonX.Y or py -X.Y depending on the platform. But I don’t know how is best to annotate each code block to make it obvious to the user they should not copy-paste the command verbatim and must read the block at the top.
|
Bah, I'm not super keen on this. I really think what we need is better capabilities in our documentation, to provide more platform specific instructions in our documentation overall without looking ugly/tacked-on (see https://squidfunk.github.io/mkdocs-material/getting-started/#with-docker-recommended for a good example). |
|
@pradyunsg - that will be really cool!! |
|
Looking around I found sphinx-tabs. This might be helpful to add to the documentation. |
Yeap -- that's the ideal thing for us to be using. I don't like the out-of-the-box styling of it TBH, so I'm gonna defer to others on the how of this. :) |
|
Agreed sphinx-tabs looks like a good way to go (if we could add some browser detection to default to the tab for the reader's OS, then even better!). And I also agree that the default style isn't ideal. But I'm no css expert so I'm not going to be the one to change it 🤷 Also, on the importance of giving a correct command for the user's situation, we're seeing more and more reports of " ¹ Don't get me started on my opinion of that particular bit of behaviour 🙁 |
|
Here is another sphinx extension which I think looks better- https://sphinxcontrib-contentui.readthedocs.io/en/latest/index.html |
|
@pradyunsg - Should I close this PR and open a new issue to add tabs to the documentation? |
|
I added the Let me know what you guys think. |
|
My expectation was that the Unix instructions would be On a more subjective note, I find the style you posted in the screenshot too distracting. Is there an alternative style that's more unobtrusive? |
|
Can we stick to sphinx-tabs? I'm happy to add a tiny bit of CSS myself to make things look better. |
Oops.. I was trying to showcase the tabs. Sorry for missing the real point. Detecting the User's OS is not there and I don't see any options to customize the look. The only way to change the look would be to change the css. |
We deploy the documentation for each PR: https://pip--8589.org.readthedocs.build/en/8589/ |
|
I run the docs build using
.......
Thats why I havn't pushed this change yet. I have edited docs/html/conf.py to add the sphinxcontrib.contentui extension in the extensions list at the end. I also added this as a docs build requirement under tools/requirements/docs.txt. What else am I missing? |
Sure. I will try and add it. |
|
If you're changing dependencies for the tox job, you'll have to pass -r as well, to tell it to recreate the environment. |
Thats it!! That Solved the issue! 🙏 |
|
You can now see the tabbed version based on sphinx-tabs here - https://pip--8589.org.readthedocs.build/en/8589/installing/#upgrading-pip I am not sure what is causing the man pages build to fail. As you can see, the html pages get built fine. Here is the log for man pages section - docs run-test: commands[1] | sphinx-build -W -d /home/srao/projects/pipdev/pip/.tox/docs/tmp/doctrees/man -b man docs/man docs/build/man -c docs/html looking for now-outdated files... none found The manual pages are in docs/build/man. Warning, treated as error: Any thoughts? |
|
I found the issue in the sphinx-tabs extension. I have requested a PR there to fix this issue. Man pages are not setup to be a build target in that project and is setup to throw a warning when a new target is used. Currently they have the following targets - |
|
@pradyunsg - is it ok for the sake of moving forward that I forked the sphinx-tabs project and make the necessary changes and reference my forked version in this PR temporarily? This will also let me play with the css/js to fine tune the look of the tabs. |
I'm not sure I follow -- what issue are you working around? To answer your question more directly, no, I don't it'd make sense to have a fork to this. It's possible to add/override a CSS file with |
As suggest in the sphinx-tabs PR, I have closed it. Looking at the history of the sphinx-tabs project, the warning is thrown on purpose to only support platforms the tabs work on. I am not sure if one of the two is possible
|
Cool. I wasn't aware of it. |
|
Thank you for being patient with my changes as I am still learning the ropes. If what I did by creating a new conf.py for man pages is acceptable, then I can trim the config files even more to be specific to each type of build. Please let me know if I am going in the right direction. |
|
I was reading the documentation for sphinx-build and saw that I can override settings using the -D option. so I will undo the last changes I made and just resubmit with just the tox.ini changes to use the -D for man build. |
|
Here are some designs I came up with - Any preferences? |
|
Personally I prefer (3). |
pradyunsg
added
type: docs
|
It's fine -- Travis CI is acting up. |
|
At the cost of being a little/very annoying... I'd like us to defer adding sphinx-tabs to our current docs, since sphinx-panels added the same functionality (and I recently added a bug there!). That approach to tabs can be stylized to look real nice much more easily! Example from Furo's documentation:
|
|
Personally I'd favor sphinx-tabs though, for the reason mentioned in the docs @pradyunsg linked:
Regarding the look, I think we can work with upstream to see if there's some option, but I'd consider being able to select the OS just one a must-have feature. |
I'm an upstream maintainer/collaborator on both of those packages. :) |
|
Is there any technical hindrance preventing sphinx-tabs to be styled (TBH I'm fine with the way it looks ATM)? Also the entry point invocation issue has caused quite many issues since I started watching this repo that I think it'd be really nice to have this merged as soon as possible, especially when @shireenrao has put a lot of effort following up on the patch. Edit: to clarify, by really nice, I meant being nice for the users to not run into confusing situations. Of course it'd be nice for us too that we will spend less time helping troubleshooting those 😈 Regarding furo, as much as I like it and expecting it to hit stable ASAP, let's be realistic that you're a quite busy person and usually over-work yourself. Tying this with furo would lead to one of the following situations
|
I'm not 100% sure what you meant here, but the current version lets you select the OS once per page, but if you go from (for example) "Installing" to "User Guide", you need to re-select your preferred OS. For Unix users not such a big deal (as it's the default) but annoying for Windows. Two possibilities here:
Currently, this PR feels like it's "nearly there". The defaulting isn't perfect, and we can argue styling all day (I'm not massively keen on the current choice of style for example, but 🤷 ) If the OS selection/persistence issue can be corrected, I'd like to see that. If it's hard, then I'd like to see a follow-up issue raised saying that it needs fixing, so it doesn't get forgotten. But let's not make perfect be the enemy of good. I'd be fine with merging this and doing subsequent work (including something like switching the implementation to |
|
Thank you for the correction, I was arguing that design-wise sphinx-panels wouldn't support tab-group equivalence. I'll file the issue on site-wise persistence just in case it is forgotten a few days after this getting merged. |
|
Let's merge this in. This PR is an improvement over status quo, and any potential future changes should likely build up on top of this one. :) |
|
I was playing around with the look and had posted some styles earlier here. As for the issue with remembering the tabs across pages.. there was an issue opened a few weeks ago which should bring this feature in. |
|
The issue you've linked to is blocked on me doing a rewrite of the sphinx-tabs package, and I certainly plan to have better styling by default when I get to it. It might just end up being that I do a rewrite that fixes all these issues for us. 🤷🏻♂️ |
|
Thanks @shireenrao! ^>^ |
|
Thanks so much for pushing this through @shireenrao! |
|
I am glad this worked out. |
|
There seems to be a problem on the Quickstart guide. For some reason the tabs are not rendering on that page. The PR build version is showing it. |
|
Good work! I see the tabs on the Quickstart page.
You could try clearing your browser cache or using Incognito? |
|
Its working now. What was weird was that the tabs were fine in the other pages except the quickstart page. I had tried it on multiple browsers and devices. |
This PR is to resolve #7311. I will change a few pages at a time and submit them to this PR.
(Edit by @uranusjr so a merge would auto-close to issue.)