Skip to content
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

Put docs in main repo, working API reference #170

Merged
merged 32 commits into from
Jul 30, 2021
Merged

Put docs in main repo, working API reference #170

merged 32 commits into from
Jul 30, 2021

Conversation

pdurbin
Copy link
Contributor

@pdurbin pdurbin commented Jul 7, 2021

Closes #148

As part of this pull request, I went ahead and incorporated the Python docs @Shoeboxam added but rather than using his "build docs" script, I tweaked the Sphinx config file. Here's how it looks:

Screen Shot 2021-07-07 at 3 57 37 PM

A few things to note:

  • If you'd like to try this branch locally, make html should work fine but if you want to try make versions you'll have to go into source/conf.py and change "main" to "148-docs" in the line that says smv_branch_whitelist = r'^main$'
  • Right now I have smv_branch_whitelist set to "main" but somehow I'd like the docs for "main" to appear at a URL for "latest". To achieve this in the docs repo, we used "latest" rather than "main" as the default branch but I'm hoping we can avoid this.
  • I'm including the GitHub Action we use in the docs repo but I changed it (and the redirect page) to "main" instead of "latest". I haven't get set up the gh-pages branch, CNAME, etc. For now the CNAME (in the GitHub Action) is docz.opendp.org until we decide we're happy with this setup of having the docs in the same repo.
  • Since I'm not using Mike's "build docs" script, I think we'll have to manually create files like opendp.v1.meas.rst when we add a new module. These are basically small index files and if this becomes a problem, we can probably figure out some automation.

@pdurbin pdurbin marked this pull request as draft July 15, 2021 16:06
@pdurbin pdurbin marked this pull request as ready for review July 15, 2021 19:58
@pdurbin
Copy link
Contributor Author

pdurbin commented Jul 15, 2021

I'm switched this back to a non-draft pull request.

First, you can now preview the live site here: https://docz.opendp.org/en/148-docs/index.html

Yes, one we merge, we'll change "docz" to "docs" and make it the live site.

I still don't have a solution for "latest" vs "main". The new site (this pull request) will use "main", the default branch. I did open an issue upstream today: Holzhaus/sphinx-multiversion#78

cp -r /tmp/build/html/* .
git add --all --force
- name: Push docs to gh-pages branch
if: success() && github.ref == 'refs/heads/main'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How often do we want to update the documentation? Per-merge or per-release?
In this PR #162, CI will release a new python package and crate whenever there is a push to the release branch. If you tie documentation deployment to the release branch, then the documentation will update less frequently, only when a release is cut.

Copy link
Member

@Shoeboxam Shoeboxam Jul 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it could update the docs each merge into main, where the updated docs includes both the latest branch (renamed from main) and all of the tags.

Within the CI script, why can't you git branch -D latest && git checkout -b latest before building to make the generated documentation look as if it were on the latest branch?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least for the time being, while docs are being worked on quite actively, I was thinking the docs site should be updated when pull requests were merged.

In this pull request, the updated docs would appear under "main" (docs.opendp.org/en/main), reflecting the fact that the pull request was merged into the main branch.

Assuming a regular release cycle in the future, I'm open to only updating docs on release. That's what we do for Dataverse. That said, even in the context of Dataverse, when we haven't released for a while, I'm often interested in seeing what the updated docs will look like, so I think there's value in always having "main" or "latest" to be able to preview what's coming in the next release.

I guess it could update the docs each merge into main, where the updated docs includes both the latest branch (renamed from main) and all of the tags.

I'm not sure I fully grok this. Is renaming main on the table? The extension I'm using does support tags. No problem there. That's my plan for people being able to switch between different versions of the library.

I don't think the checkout above will work but in the same vein, we might be able to have the CI whatever is in main to a branch called latest and then run make versions. I haven't tried this yet.

Copy link
Member

@Shoeboxam Shoeboxam Jul 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, if we don't rename main, then each time main is pushed to, CI could run something like:

git branch -D latest && git checkout -b latest
make versions

...where smv_branch_whitelist = r'^latest$'.
By "rename from main" I was referring to the checkout. I don't think I'm the guy to decide if we will rename main. I agree that it makes sense to push docs more frequently. I'm ambivalent on the naming, but wanted to share a method for how you might go about changing the version name without making repository changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see. Delete the latest branch and then create it again from main. Yeah, I would hope that would work. I was thinking of a merge from main to latest but I'm not picky. 😄

Copy link
Member

@Shoeboxam Shoeboxam Jul 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just brainstorming. Merges would work too.

# built documents.
#
# The short X.Y version.
version = '0.0.1'
Copy link
Member

@Shoeboxam Shoeboxam Jul 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was attempting to add some global checks to make sure version numbers across projects are synchronized in the CI release PR. There's a root-level VERSION file and a .github/assert_version.py that checks version numbers. I'm not sure if that's how we want to handle things, but if it is, maybe this could read the version from the VERSION file? Perhaps the release CI script could upload a tag based on the contents of that VERSION file too, so we always know the VERSION file is synchronized with the tag.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I absolutely want us to have a single source of truth for the version. I just threw 0.0.1 in there for discussion. I'm glad you found it. Reading from a file should work, I would imagine.

@pdurbin
Copy link
Contributor Author

pdurbin commented Jul 19, 2021

I just made several commits to get the docs previewing again at https://docz.opendp.org/en/148-docs/index.html

This seems necessary if we're going to work on content and various rearranging, as @andrewvyrros has proposed.

Before merging, we'll need to revert a number of commits. I thought 66bd776 would be enough to get previews working again. I don't think 21cfbe4 was actually necessary for me to commit but I kept getting "No matching refs found!". In the end
8c91ef5 got the build working (got that error to go away) which is very strange. It's as if sphinx-multiversion only works when the branch you want to build is specified under "branches" in the GitHub Action. Long term, this is not what we want. We want to be able to do builds of pull requests, not just builds from merges to main.

Comment on lines 13 to 32

def _get_lib_dir() -> str:
lib = None
if os.environ.get('OPENDP_HEADLESS', "false") == "false":
lib_dir = os.environ.get("OPENDP_LIB_DIR", os.path.join(os.path.dirname(os.path.abspath(__file__)), "lib"))
if not os.path.exists(lib_dir):
# fall back to default location of binaries in a developer install
build_dir = 'debug' if os.environ.get('OPENDP_TEST_RELEASE', "false") == "false" else 'release'
lib_dir = os.path.join(os.path.dirname(os.path.abspath(__file__)), *['..'] * 4, 'rust', 'target', build_dir)
if not os.path.exists(lib_dir):
raise ValueError("Unable to find lib directory. Consider setting OPENDP_LIB_DIR to a valid directory.")
return lib_dir


def _get_lib_name() -> str:
platform_to_name = {
"darwin": "libopendp_ffi.dylib",
"linux": "libopendp_ffi.so",
"win32": "opendp_ffi.dll",
}
if sys.platform not in platform_to_name:
raise Exception("Platform not supported", sys.platform)
return platform_to_name[sys.platform]

lib_name = platform_to_name[sys.platform]

lib_path = os.path.join(_get_lib_dir(), _get_lib_name())
lib = ctypes.cdll.LoadLibrary(lib_path)
lib = ctypes.cdll.LoadLibrary(os.path.join(lib_dir, lib_name))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something seems funky here. Are these changes intended, or is this a merge issue?

Copy link
Member

@Shoeboxam Shoeboxam Jul 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was something I added. The docs build sources the library. If the binaries don't exist, the sourcing fails, so the docs fail to build. So he had to start building the rust binaries from the docs workflow to get the docs to build. This makes it so that if the env var OPENDP_HEADLESS is set, then the binaries are not loaded, bypassing the need to build the binaries.

@pdurbin
Copy link
Contributor Author

pdurbin commented Jul 20, 2021

This seems necessary if we're going to work on content and various rearranging, as @andrewvyrros has proposed.

I just pushed the content reorg @andrewvyrros and I have been talking about: https://docz.opendp.org/en/148-docs/index.html

Please note that many of the pages are blank but the structure is there.

Even thought this pull request is a mix of content and deployment, these topics can be discussed separately.

For content, I'd like to bring up the following comments or questions

  • Now that we have a "Resources" section, how should we treat the various links at Resources page in User Guide docs.smartnoise.org#28 ? Perhaps some of the links can go under the page called "Tutorials". Do we need more pages under "Resources" for these links?
  • I had a page called "Making Releases" but didn't see this in the new reorg so I took it out. I still think it's valuable to document your release process but perhaps it's premature since we haven't made a release yet.
  • I had a page in that talked about various aspects of contributing with topics like Roadmap, Kanban Board, Issue Tracker, Getting Help, Other OpenDP Projects. Do we want to add some of this back in somewhere?
  • DP Creator now has its own page. Is this necessary? It's also listed on the main OpenDP Commons page.
  • Related to OpenDP Commons, I renamed index to intro to make the hierarchy work but it seems odd. I could use another pair of eyes here.

I have lots of thoughts on deployment and CI too:

  • Should we be pushing the HTML to a different repo? Right now it goes to the gh-pages branch.
  • Do we like having "en" in the URLs? Is this overkill? Will we ever have translations?
  • Where should we redirect the root to until stable docs exist? "en/latest"?
  • sphinx-multiversion has been a mixed blessing. I may want to drop it but we'll need some sort of strategy for versioning, either building our own solution or hosting with a place like Read The Docs.

@andrewvyrros
Copy link
Member

andrewvyrros commented Jul 20, 2021

Thanks Phil, this is coming together!

  • Now that we have a "Resources" section, how should we treat the various links at Resources page in User Guide docs.smartnoise.org#28 ? Perhaps some of the links can go under the page called "Tutorials". Do we need more pages under "Resources" for these links?

Yeah I think it makes sense to have separate pages under Resources. I think most of the stuff in that issue could fit in a page called References or Educational Material or similar. For Tutorials, I was imagining that being just coding tutorials (like end-to-end instructions focused on a single use case.)

  • I had a page called "Making Releases" but didn't see this in the new reorg so I took it out. I still think it's valuable to document your release process but perhaps it's premature since we haven't made a release yet.

For sure we want to document that. It just felt a little early to have that in our public docs, as we're still inventing it, and don't want to be encouraging people to try to cut their own releases! (@Shoeboxam discovered what seems like an access hole in GH repo permissions around releases.)

  • I had a page in that talked about various aspects of contributing with topics like Roadmap, Kanban Board, Issue Tracker, Getting Help, Other OpenDP Projects. Do we want to add some of this back in somewhere?

The issue tracker and project board stuff is a little tricky, I think that's worth a separate discussion.

For Getting Help, I was thinking that kind of stuff could live under Contact. Alternatively, we could have the top-level module be labeled Help, and have various ways of getting help under there?

  • DP Creator now has its own page. Is this necessary? It's also listed on the main OpenDP Commons page.

Yeah I guess it makes sense to have it live under OpenDP Commons.

  • Related to OpenDP Commons, I renamed index to intro to make the hierarchy work but it seems odd. I could use another pair of eyes here.

Yeah something's a little funky there. We probably don't need the second level OpenDP Commons node. Maybe the hierarchy looks like this:

  • OpenDP Commons
    • What is the OpenDP Commons?
    • List of OpenDP Commons Tools and Packages
      • OpenDP Library
      • DP Creator
    • Related Projects
      • Google Differential Privacy
      • Additional Projects
  • Should we be pushing the HTML to a different repo? Right now it goes to the gh-pages branch.

I was thinking it'd be cleaner to push the HTML to a separate repo, and not have publishing artifacts in the main repo. But I've never use GH Pages before, so if there's a compelling reason to keep it in a single repo, I'm open to that.

  • Do we like having "en" in the URLs? Is this overkill? Will we ever have translations?

It's probably a bit aspirational, but doesn't seem to harm anything?

  • Where should we redirect the root to until stable docs exist? "en/latest"?

Yeah latest seems reasonable.

  • sphinx-multiversion has been a mixed blessing. I may want to drop it but we'll need some sort of strategy for versioning, either building our own solution or hosting with a place like Read The Docs.

Yeah there's always a tradeoff with adopting tools like that. If we have to roll our own solution to get the best docs, then so be it.

pdurbin and others added 18 commits July 30, 2021 10:06
See also the following in source/conf.py

TODO: We would rather have "latest" than "main".
smv_branch_whitelist = r'^latest$'
This reverts commit 5471219.

Reverting because we started seeing "No matching refs found!" again.
Right now "docs" is copied to the root, along with "rust" and "python/src/opendp/v1/ __pycache__".
An "if" in the "push to gh-pages" prevents updating the live site except
on merge.

For now, whitelist only "main". Link to new issue about "latest".

Also get rid of mentions of 148-docs branch.

TODO: change docz CNAME to docs.
Co-authored-by: Michael Shoemate <shoeboxam@gmail.com>
Co-authored-by: Michael Shoemate <shoeboxam@gmail.com>
Just trying to get the docs to build. Right now we are seeing
"No matching refs found!"
pdurbin and others added 3 commits July 30, 2021 10:06
…he-fly with sphinx-apidoc.

* Pointed to Rust API docs on docs.rs.
* Removed extra Python path entries.
* Updated docs/requirements.txt for sphinx 4.1.2, patched sphinx-multiversion.
* Cleaned up Makefile for single-version case.
* Updated triggers for docs workflow.
* Added sync-branches workflow to sync stable and latest branches.
* Rebased to main.
@andrewvyrros
Copy link
Member

I've made it so that the API doc structure is generated dynamically for each release, so we don't have to keep the .rst docs in sync with Python packages. I also came up with a strategy for handling stable and latest branches.

Copy link
Member

@Shoeboxam Shoeboxam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Woohoo! Nice to have in-repo docs! Thanks Phil! Thanks Andy!

@Shoeboxam Shoeboxam merged commit c1add41 into main Jul 30, 2021
@andrewvyrros andrewvyrros deleted the 148-docs branch August 30, 2021 23:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

docs in OpenDP library repo
3 participants