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 #95

Merged
merged 6 commits into from
Nov 24, 2022
Merged

Documentation #95

merged 6 commits into from
Nov 24, 2022

Conversation

hbcarlos
Copy link
Contributor

@hbcarlos hbcarlos commented Nov 24, 2022

Done

  • Adds Sphinx
  • Sphinx Autodoc for Python API docstring
  • TypeDoc for JavaScript API docstring
  • Configure Read The Docs

TODO:

  • Write an overview
  • Document schemas
  • Explain how to create a custom model
  • Write docstring for the python package
  • Check docstring for the javascript package

Screencast

Screencast.from.11-24-2022.10.45.24.AM.webm

@davidbrochart
Copy link
Collaborator

Wow, that's great, thanks a lot Carlos!

@hbcarlos
Copy link
Contributor Author

@davidbrochart, Do you know if I can ignore the docs folder for the link_check workflow?

@hbcarlos hbcarlos marked this pull request as draft November 24, 2022 09:38
@davidbrochart
Copy link
Collaborator

Maybe do the same as jupyter-server/jupyter_server@05913dd ?

@davidbrochart
Copy link
Collaborator

Maybe do the same as jupyter-server/jupyter_server@05913dd ?

Ah, it's already there.

@hbcarlos
Copy link
Contributor Author

hbcarlos commented Nov 24, 2022

Thanks, @davidbrochart!

Yes, it is already there. Anyway, I'm reorganizing the index to remove the failing links, and I can maybe build the docs before checking the links for the remaining link.

@davidbrochart
Copy link
Collaborator

davidbrochart commented Nov 24, 2022

But link_check is actually failing for a good reason, right? For instance, there is no ./api/index.html yet.

@hbcarlos
Copy link
Contributor Author

Yes, because that link points to the TypeDoc documentation for the javascript package that is generated when building the docs. That link will always fail, we need to ignore it.

@davidbrochart
Copy link
Collaborator

Ah I see, I don't know if it's possible to exclude a single link.
Otherwise, why not have an empty file, that will be overwritten when building documentation?

@hbcarlos
Copy link
Contributor Author

Yeah, that is a solution. I can generate that file when building the documentation.

@hbcarlos
Copy link
Contributor Author

@davidbrochart. Much easier, I added a placeholder file, so the link doesn't fail.

@davidbrochart
Copy link
Collaborator

I can see that when clicking on JavaScript API, it goes to a different layout, without the top banner. Is it expected?

@hbcarlos
Copy link
Contributor Author

hbcarlos commented Nov 24, 2022

I can see that when clicking on JavaScript API, it goes to a different layout, without the top banner. Is it expected?

Yes, it is the same JupyterLab does.

It is because I'm using TypeDoc to automatically generate the documentation of the JavaScript package from the docstring. That generates a completely different webpage that I copy to the build folder of the Sphinx documentation, see:

"docs": "typedoc --out ./docs src",

and:
# Build JavaScript Docs
js = HERE.parent.parent / "javascript"
js_docs = js / "docs"
dest_dir = Path(app.outdir) / "api"
if js_docs.exists():
# avoid rebuilding docs because it takes forever
# `make clean` to force a rebuild
print(f"already have {js_docs!s}")
else:
print("Building jupyterlab API docs")
check_call(["npm", "install"], cwd=str(js))
check_call(["yarn", "run", "build"], cwd=str(js))
check_call(["yarn", "run", "docs"], cwd=str(js))
# Copy JavaScript Docs
print(f"Copying {js_docs!s} -> {dest_dir!s}")
if dest_dir.exists():
shutil.rmtree(str(dest_dir))
shutil.copytree(str(js_docs), str(dest_dir))

There is an extension for Sphinx, sphinx-js, that does everything inside sphinx, but it doesn't work yet with the jinja2>=3. See: mozilla/sphinx-js#191
And the latest Sphinx uses jinja2>=3. In addition, jupyter-server, jupyterlab, and some other packages that we need to install in the same env all use jinja2>=3, soo it makes everything more complicated for development.

@hbcarlos hbcarlos marked this pull request as ready for review November 24, 2022 10:31
@hbcarlos
Copy link
Contributor Author

I would like to merge and do the remaining tasks in follow-up PRs. This way, we can set up the Read The Docs project and check that everything works.

@davidbrochart davidbrochart added the documentation Improvements or additions to documentation label Nov 24, 2022
@davidbrochart davidbrochart merged commit 4612786 into jupyter-server:main Nov 24, 2022
@davidbrochart
Copy link
Collaborator

Thanks!

@hbcarlos hbcarlos deleted the docs branch November 24, 2022 10:54
@hbcarlos hbcarlos mentioned this pull request Nov 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants