-
Notifications
You must be signed in to change notification settings - Fork 951
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
Sphinx: Link each entity to its source code on github #262
Conversation
Looks good! It would be good to make it differentiate between master and release already, I have a feeling we'll forget about it if we wait until the release. |
Please have a look at the numpy code: They solve this via
Yes, in any case I wouldn't merge this without copying in the code I linked to. Without links to the correct line number it would be a step back from what we have. |
e6498fc
to
faf5e2f
Compare
Rebased and amended. It now links to the correct line number in the correct file in the correct branch/tag. Ready for reviewing and merging. |
# The full version, including alpha/beta/rc tags. | ||
release = '0.1' | ||
release = lasagne.__version__ |
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.
This saves another version string update that could easily be missed; numpy does it the same way. Let's see how readthedocs thinks about that. I've read that you may need to configure readthedocs to use virtualenv
.
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 would it work with virtualenv
but not otherwise? I'm not sure how it's currently set up - there seem to be multiple virtualenv-related settings, some of them are enabled and some aren't. It's a bit unclear what they all do. I remember having to tinker with them for a while before everything would work, though.
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.
This line requires "import lasagne" beforehand, maybe this is not universally fine. I also don't know exactly how Sphinx works. We can just merge and see what happens.
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.
Fine by me :)
Not much to say about this as I'm not very familiar with how Sphinx works. Everything looks fine to me. |
Sphinx: Link each entity to its source code on github
Okay, merged and crossing fingers. /edit: It works! |
Wait, it doesn't work for |
This links each entity to its source file on github. This should make it a lot easier for people to suggest edits to docstrings.
Numpy tweaked this further (https://github.com/numpy/numpy/blob/master/doc/source/conf.py#L286), in two respects:
It should be possible to copy and adapt their code, but I don't have the time now -- I'd welcome any PRs to my PR!