Initial Pass at Lyse docs

[dihm]

I was inspired by conversation here and figured I would get things started by doing the basic minimum, since docs are much easier to edit that write from scratch.

So I basically just copy-pasted from the pdf doc Chris wrote for lyse and added the core API documentation pages. I've allowed sphinx to document methods without docstrings for now so it is more obvious what methods need them ;)

[dihm]

This is my first pull request on github so I imagine there is going to be a bunch of procedural errors that will need fixing, even on such a trivial change as this.

[philipstarkey]

This is my first pull request on github so I imagine there is going to be a bunch of procedural errors that will need fixing, even on such a trivial change as this.

Hi David,

Thanks for the work on docs!

It looks like you might have based this on the 3.0.x maintenance branch rather than the development (master) branch. This means that some of the commits from the master branch we previously cherry picked/squash merged over to the maintenance branch appear to be coming back to master again in this PR.

I think the solution is to rebase your single commit onto master and then force push to your remote repository (to overwrite the previous push, which should also update this PR I think...). However this might not work nicely and/or could put your git repo in a messed up state if I'm wrong, so I recommend making a backup first before trying anything. Worse case, you could copy the modified files in your commit somewhere, update to master, paste them back into the repo, and push a new commit to a new branch and make a new PR. But I think you can avoid that in theory.

@chrisjbillington might have a better suggestion too.

[dihm]

You are quite right! I should have known to work off master.

Anyway, the force push seems to have sorted that out. Let me know if there is any borkiness with the docs themselves.

[philipstarkey]

You are quite right! I should have known to work off master.

Anyway, the force push seems to have sorted that out. Let me know if there is any borkiness with the docs themselves.

Great! We have an integration with github pull requests and readthedocs, so your changes should be automatically built soon and we can review (seems to be taking a while, but maybe that's because this PR was built recently from the first set of commits). The link to view will appear below once the 1 pending check has completed!

[philipstarkey]

Cool, it finally built!

Looks generally good! Got a couple of questions:

1. The "Analysis Subprocess" page is empty. Is this intended or did something go wrong with the autodoc?

2. The introduction page has a author attribution to "Chris Billington" in the source code. This doesn't show up in the built docs. It might not matter, probably depends on where the content came from. Is this something Chris wrote somewhere (old docs, thesis, something else)? My thoughts are we only need these sorts of attributions if it's coming from a thesis (but others may have differing opinions)

3. There is a footnote on the introduction page. The actual footnote itself looks a bit messy (not really clear the text belongs to the footnote). Not sure if this is standard formatting for footnotes on readthedocs or if there is something we can easily do to make it look nicer?

[dihm]

1. That is odd. I actually tested everything in a local sphinx build environment and it was working there. I'll check again tonight (the build computer is at home). Could autodoc be having trouble with the Qt imports? Is there an easy way to check the build logs?

2. I put that in because I lifted the intro docs directly from the pdf doc that comes with lyse that @chrisjbillington wrote back in 2014. Apparently those author tags don't show up unless a build option is set. I figured since it wasn't a thesis, but still something that originally had an author attribution, I would compromise and at least put it somewhere invisible in the source. I'm happy to do whatever with it.

3. Apparently that is the default for footnotes on readthedocs. I agree it isn't compelling. I can either make a footnote header so it is more obvious what is going on, or I can just roll the note into the main text since there is only one footnote anyway. I lean towards the latter personally.

[philipstarkey]

1. Build log is here (FYI all build logs, PRs or otherwise, show up in the readthedocs project's build tab). There is an error to do with failing to import the site module. Feel like I've seen that before...but it's also triggered by a line in @chrisjbillington desktop app library in this case... Don't have time to look into it now (going to sleep), feel free to debug or not (we can figure it out if you don't want to waste the time).

2. Sounds good to leave as is then.

3. Whichever you like. I'm not fussed!

[dihm]

It looks like this is a long standing issue with virtualenv. As I understand it, virtualenv ships its own implementation of site.py that is not forward compatible (ie missing functions) with newer versions of python. When you try to use the site module, it finds the virtualenv version instead of the one shipping with python. Given that the issue still exists 8 years later, I suspect it isn't going to be fixed and an alternative, or hack, is necessary.

Edit: Checked on my local machine and sphinx builds the Analysis Process page just fine.

[philipstarkey]

@chrisjbillington The call to desktop_app.set_process_appid('lyse') in analysis_subprocess.py is preventing the lyse docs for that file from building. Would it make sense to move that line (and the import of desktop-app above it) inside the existing if __name__ == '__main__': block? Or is there some subtlety with how the analysis subprocess is launched with zprocess that would make that a bad idea?

@dihm Ah, I remember the issue now that you've explained the source of it. Just importing desktop_app used to trigger this issue, which we fixed. But in this case it's the actual use of the desktop-app package at import time that's the problem! Imports shouldn't really be doing much generally so I suspect the fix is to solve that rather than trying to hack a fix for the site module. But we'll see what Chris says.

[philipstarkey]

In the interest of keeping things moving, I'm going to merge this and log a bug about the analysis_subprocess page. We can fix it later (since everything builds except for that API page).