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

Documentation page build action using Sphinx #14

Merged
merged 21 commits into from
Aug 1, 2023

Conversation

slobentanzer
Copy link
Collaborator

The new workflow uses sphinx on a GitHub action runner to create html documentation, including an API reference generated from the docstrings, and pushes it to the gh-pages branch.

To deploy the webpage, the repository needs to be configured for hosting a gh-pages website in the settings ("Pages" > from branch).

@slobentanzer
Copy link
Collaborator Author

This is the page on my GitHub: https://slobentanzer.github.io/sctk/index.html

@ktpolanski
Copy link
Contributor

ktpolanski commented Jul 31, 2023

  1. Can the demo notebook be included as a tab?
  2. Can the API reference be limited specifically to sctk.calculate_qc(), sctk.generate_qc_clusters(), sctk.cellwise_qc(), sctk.clusterwise_qc() and sctk._pipeline.fit_gaussian()? Bonus points if they come in the form of clickable sub-pages, Scanpy docs style.

@slobentanzer
Copy link
Collaborator Author

Can the demo notebook be included as a tab?

As an executable, i.e. colab? I think if you do not want to host that yourself, would have to be a link rather than embedded. But a tutorials page with links to the notebooks is easy.

Can the API reference be limited

Not sure, I am not a Sphinx wizard. But we can definitely find that out.

Bonus points if they come in the form of clickable sub-pages

I think they use the autosummary Sphinx extension for that, we can probably implement that fairly easily, although I have not worked with that before.

@ktpolanski
Copy link
Contributor

Yeah, the notebook as a static render. Exactly as the current GitHub link does, but within the docs.

I specify :members: in the BBKNN docs and it seems to only do those? https://raw.githubusercontent.com/Teichlab/bbknn/master/docs/index.rst

@ktpolanski
Copy link
Contributor

Look at that, it did the thing. It's marginally annoying that it's in two tables but it kind of reflects on the fact the GMM is a utility function, I guess.

I'm having no luck trying to get the notebook to show up. I moved the notebooks folder into docs, added "nbsphinx" as an extension. I get stuck on WARNING: toctree contains reference to document 'notebooks/automatic_qc' that doesn't have a title: no link will be generated and it refuses to show up. Gotta love Sphinx.

@slobentanzer
Copy link
Collaborator Author

If you want folks to use the gaussian utility function, why not put it in the __init__.py as well? Then you don't need the separation in the docs. IMO all user-facing functions should be in the interface, anyways.

@ktpolanski
Copy link
Contributor

ktpolanski commented Jul 31, 2023

As stated before, its behaviour can be controlled via the **kwargs of sctk.cellwise_qc(), so it's important to show what those arguments actually are. But I'm not sure it's important enough to merit an actual sctk. import.

Hopefully you'll have better luck with that damned notebook than I did. I tried adding a title cell and everything.

@slobentanzer
Copy link
Collaborator Author

What about just linking it from a regular markdown page ("example notebooks" or "tutorials")? Could be an adequate short-term solution.

@ktpolanski
Copy link
Contributor

Can you help me with the nbsphinx in the toml file? I have no idea what the hell to do it to clear the automated builds, I thought "^0.8.9" meant "0.8.9 or newer". I know I just pipped it in here when testing and it worked fine. I'm sorry. I'm not a poetry guy. In flit you can just specify a package without versions :(

@slobentanzer
Copy link
Collaborator Author

Not sure what you mean, in poetry you don't have to specify versions. I never worked with that either, and I rarely work with notebooks. Do you have a working example of how you want it to look?

@ktpolanski
Copy link
Contributor

My query has nothing to do with the notebook part. It has everything to do with the cursed toml and lock and I don't know what. Building is failing and I have no idea why. Help.

@ktpolanski
Copy link
Contributor

I tried adding just the line nbsphinx to the toml, to what it wasn't having it, it needed a version constraint too. So I ballparked 0.8.9 based on the age of some other package's constraint on pip, and everything remained exploded.

Literally all I've done is added nbsphinx = "^0.8.9" to the dev dependencies in the toml and it's not building anymore.

Please help :(

@slobentanzer
Copy link
Collaborator Author

Keep calm and read the docs. :)

Have you removed your venv and lock file and installed from scratch yet? For me it builds without problems with nbsphinx.

@ktpolanski
Copy link
Contributor

@slobentanzer
Copy link
Collaborator Author

It says right there in the error:

Warning: poetry.lock is not consistent with pyproject.toml. You may be getting improper dependencies. Run `poetry lock [--no-update]` to fix it.

-> remove lock file

@ktpolanski
Copy link
Contributor

You've always had it, and made small incremental changes to it, so it felt like it was not my place to remove it. But if you say so.

@slobentanzer
Copy link
Collaborator Author

The lock file is not that important, you can simply regenerate it by running poetry install. It is good for reproducibility, but in development it is expendable.

@ktpolanski
Copy link
Contributor

Thanks for the help. The docs have rendered sensibly online now.

If you have opinions about nbsphinx's version, feel free to change it. If you have opinions that the lock file should return, feel free to reintroduce it.

Should there be a badge about the docs?

@slobentanzer
Copy link
Collaborator Author

Thanks, looks great indeed. Gonna use this for my other repositories from now on. :)

If you have opinions that the lock file should return, feel free to reintroduce it.

I did not actually mean to completely remove it at all, but rather temporarily, just to fix the issue. It is good practise to provide it in case someone wants to reproduce that exact version. I'll add it back.

@ktpolanski
Copy link
Contributor

Sure thing.

I think this just about does it! Minimal changes left to the readme, linking some stuff and whatnot. Do I need to change anything in the docs configs or will they spawn at https://teichlab.github.io/sctk/ automatically?

Do you know how to tell the poetry TOML to render the readme on the pip page, and have links to stuff? I know how to do it in flit, which I'm not sure is that useful here

@ktpolanski
Copy link
Contributor

Turns out the readme and various links can be easily accomplished: https://python-poetry.org/docs/pyproject/

I can sort it out on my end once you're happy with how things are here and I merge?

@slobentanzer
Copy link
Collaborator Author

Do I need to change anything in the docs configs or will they spawn at https://teichlab.github.io/sctk/ automatically?

You need to set up what is described in the PR description above.

TOML to render the readme on the pip page, and have links to stuff?

Do you mean pypi.org? It just uses your GitHub readme AFAIK. But maybe that is done by Poetry, IDK. It always just worked for me.

@ktpolanski
Copy link
Contributor

Is this mergeable? I presume I may have queries about the page setup once merged.

@slobentanzer
Copy link
Collaborator Author

Turns out the readme and various links can be easily accomplished

True dat, you could add some classifiers and the docs homepage. Do you want to create a separate file for the pypi readme?

once you're happy

I am good. :) Remember to replace the docs building badge location, I used my fork again for testing.

@ktpolanski ktpolanski merged commit b2c245a into Teichlab:master Aug 1, 2023
@ktpolanski
Copy link
Contributor

ktpolanski commented Aug 1, 2023

Alright so the docs merged and I updated the badge and everything.

As for the page, I found this:

image

I presume I point this to the mysterious new gh-pages branch that somehow appeared after I merged?

@slobentanzer
Copy link
Collaborator Author

It's not that mysterious :D

and yes

@ktpolanski
Copy link
Contributor

Perfect, thanks. Check it out, we're live! https://teichlab.github.io/sctk/

I'll make some minimal tweaks to the readme and the toml and then up on pip it goes. Thanks a lot for all your help with this stuff.

@slobentanzer
Copy link
Collaborator Author

Very happy to help in open source! I also learned some new things. :)

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.

2 participants