-
Notifications
You must be signed in to change notification settings - Fork 199
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
Update index of Sphinx docs #86
Conversation
sudo apt-get install python-pyqt | ||
sudo apt-get install python3-pyqt | ||
|
||
if PyQt4 is desired. |
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.
I'm not sure it's necessary to illustrate this; there's no particular reason the end user should care about using Qt 4 vs Qt 5. If they're using the Qt console as part of another Qt 4 application, the instructions for that application should get them set up.
If we do want to show it, then the names are python-qt4
and python3-pyqt4
.
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.
@takluyver Thanks for the correction. I don't feel strongly about keeping in or taking out. Since the code base still mentions Qt 4, it might be good to just leave it in but corrected.
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.
@takluyver, I'm in favor of keeping the pyqt4 instructions (and sorry for the name errors, I should have verified them first).
The thing is only showing python3-pyqt5
seems to imply that qtconsole
only works in Python 3 and with PyQt5, which is obviously not true.
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.
Maybe in the paragraph above, we should expand "PyQt or PySide" to say "PyQt5, PyQt4 or PySide".
I don't think there's any great harm in putting these in, but it seems like superfluous information. There's a lot of potential combinations of packaging system, Python version and Qt binding, and I don't want to end up having to make a great big list of possible commands. I'd rather give one example command and let people work out how to adapt it.
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.
I propose to leave the installation instructions for PyQt4 because (as you said) there's no harm in having them, but don't add further instructions for other Linux distributions and Qt bindings.
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.
How about if, in the paragraph above, we expand it to say "PyQt5, PyQt4 or PySide". The install line is introduced with "e.g.", which means that it's an example, so that seems pretty clear to 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.
Ok, fair enough :-) But then I'd like to see something really explicit, like
e.g. to install PyQt5 on Ubuntu and Python 3 you need to run
sudo apt-get install python3-pyqt5
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.
I'm going to rework this based on both of your concerns (verbosity/too much info and clarity/supporting the casual users).
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 @willingc !
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.
👍
@takluyver @ccordoba12 I've now updated the Sphinx docs for installation as well based on the content in PR #85. I broke out installation into its own rst file. In the next few weeks, I plan to loop back break up the index.rst to usage.rst and security.rst as well. |
|
||
.. toctree:: | ||
:maxdepth: 2 | ||
|
||
config_options | ||
installation |
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.
I'd put installation before config options
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.
+1
A couple of minor points about wording, but overall this looks good. |
@takluyver I've made your suggested changes. |
|
||
or:: | ||
|
||
sudo apt-get install python3-pyqt |
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.
They're still not called this! It's python3-pyqt4
and python-qt4
. ;-)
You can search on http://packages.ubuntu.com/ to check any package names.
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.
My bad. Good catch. Fixing...
Thanks ! |
Addresses #82 for Sphinx docs.