edwardchalstrey1 / seshat

This is the beginning of the Seshat Project code.
0 stars 2 forks source link

Adding documentation (Sphinx) and docstrings #164

Closed kallewesterling closed 4 days ago

kallewesterling commented 2 weeks ago

This PR includes a lot of edits:

edwardchalstrey1 commented 1 week ago

Thanks @kallewesterling this looks great! I'll have a proper review at some point next week I think. In the meantime I'll use today's Seshat meeting to check on Majid's progress with merging the work on this fork so far to the upstream repo and updating the production site. Until that's done I've been avoiding editing parts of the code that aren't directly related to the maps, but I'm hopeful we'll be at that point very soon and can do things like this updating his docstrings etc and do more regular PRs to avoid large conflicts

edwardchalstrey1 commented 5 days ago

Hi @kallewesterling I've just had a go at checking this out locally and building the docs (and aim to look at the conflicts next). I've added sphinx-rtd-theme which seems to be required: https://github.com/kallewesterling/seshat/pull/1 (in future feel free to use my repo instead of your fork when making branches/PRs)

On building the docs I'm currently getting these errors (and it seems to be either hanging now or just taking a very long time) - this all comes after [AutoAPI] Reading files... [100%] Update: this was caused by me having a venv for Pulumi in the directory structure

edwardchalstrey1 commented 5 days ago

Actually perhaps I should try setting up a new python env specifically for docs as the error seems to be caused by Pulumi A new Python env was not required

edwardchalstrey1 commented 5 days ago

Ok nice, docs building for me now - will have a proper review tomorrow 👍

kallewesterling commented 5 days ago

Great to hear!! Let me know if you run into any other problems!

edwardchalstrey1 commented 4 days ago

I have opened a new issue #175 to ensure tests can be run by actions in future, but for now I have confirmed tests run locally so will merge