-
-
Notifications
You must be signed in to change notification settings - Fork 13
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
Basic sphinx cleanup #32
Conversation
I ended up putting |
I've switched http://core.aspen.io/en/latest/ to build this branch for now. One thing I'm noticing is that the nav doesn't scroll and is weirdly expanded (all levels are listed flat, and you can expand levels with sublevels). I went looking for a related ticket in the RtD theme repo, and found this one which led me to this project where the nav behaves as I'd expect it to. What's going on, I wonder? |
I am daunted by the amount of work to do here. 😩 |
The same goofiness is present on http://postgres-py.readthedocs.io/en/2.2.1/. |
I'm following these heading styles (I saw those same ones somewhere else, too). |
@@ -80,6 +80,7 @@ def __env(envdir): | |||
def env(): | |||
"""set up a base virtual environment""" | |||
_env() | |||
_deps() |
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.
You should put that in the _sphinx_cmd()
function instead.
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.
Why? I want the dependencies to be installed when I build the environment, don't I?
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.
No, now that we use tox the env's basic task is only to bootstrap tox if necessary.
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.
Sounds like I need to bone up on tox ...
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.
/me reading https://testrun.org/tox/latest/example/devenv.html ...
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.
Alright, so now we have up to four virtualenvs: the env
one that we build ourselves, which is only used for tox, and then the .tox/py{27,34,35}
ones, which contain the dependencies for the project. If I want to interact with the project directly (run py.test
, import aspen
), then I need to use one of the tox-managed virtualenvs, not env
. 👍
Alright, I think this is good enough to review/merge as a first pass at making docs visible. The next step from here is to start refactoring the codebase to make the overall structure sensible (e.g. #32 (comment)), and then I'll want to go over the whole thing another three or four times. :-) |
@Changaco Ready for review! :-) |
I've just realized that this PR is in fact reverting the reorganization I had done in fc59892 (moving headings from python docstrings to the |
external tools like requirements files, including readthedocs
I've imported commits from AspenWeb/pando.py#572. |
, install_requires = ASPEN_DEPS | ||
, tests_require = TEST_DEPS | ||
, install_requires = open('requirements.txt').read() | ||
, tests_require = open('requirements_tests.txt').read() |
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.
Is this really sufficient? We don't need a splitlines
here?
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.
No splitlines
, it would probably fail on empty lines and comments if we naively split the lines.
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.
The simplest way to include requirement specifiers is to use the
install_requires
argument tosetup()
. It takes a string or list of strings containing requirement specifiers. If you include more than one requirement in a string, each requirement must begin on a new line.
TIL.
I remember thinking that it seemed weird to put the header in each file, and cleaner to put it in the |
I agree it's weird if it's only the header in the docstring. I see them as placeholders, though. I expect to write more docs in the docstrings. Let's see if it still looks weird after that ... |
envdir = _env() | ||
run('pip', 'install', *packages) | ||
_deps() |
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 reading up on Tox. There may be a better way to use tox to manage sphinx ...
https://testrun.org/tox/latest/config.html
https://testrun.org/tox/latest/example/general.html#integrating-sphinx-documentation-checks
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.
Harumph. If tox supported multiple requirements files there may have been something here. I'm not going to pursue this further for now, though.
The merge is yours, I was the last one to add commits. ;-) |
Okay! I'm good with your commits. Ready to merge? :) |
Heh. Crossed signals. :) |
No description provided.