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

Start of executable docs #777

Open
wants to merge 21 commits into
base: main
Choose a base branch
from
Open

Start of executable docs #777

wants to merge 21 commits into from

Conversation

ianhi
Copy link
Contributor

@ianhi ianhi commented Feb 24, 2025

By making docs executable we will be able to automatically pick up errors. Currently I'm doing this using markdown-exec because that was the only way to integrate into the existing markdown docs while also preserving the tabbing that exists in several pages. This has the downside of currently being a bit verbose in adding to the start of each executable codeblock. However I think this is fixable with some issues I've opened upstream (pawamoy/markdown-exec#77, pawamoy/markdown-exec#76 (comment)). There are two alternatives:

  1. mkdocs-jupyter

Woudl allow for writing documentation in jupyter notebooks. however, we would lose some of the pymdownx extensions (e.g. tabbing)

  1. Switching the docs build to sphinx and using myst would allow for writing everthign in jupyter as well as preserving tab behavior. However, this would require swapping the docs framework.

Have doc build fail on unexpected error:

markdown-exec currently warns on an unexpected error. We can make the docs fail by passing --strict to mkdocs build (pawamoy/markdown-exec#75)

Alternatively mkdocs-jupyter provides this already, or we can achieve the same with myst/jupyterbook

@ianhi ianhi force-pushed the ian/docs/exec-docs branch from ff2f51e to 08341c9 Compare February 24, 2025 21:02
@ianhi
Copy link
Contributor Author

ianhi commented Feb 24, 2025

Ahh annoyingly this requires readthedocs to know how to install the dev version of icechunk, otherwise it won't be able to properly execute the docs. may take some figuring out to make that happen in readthedocs with poetry etc

@@ -57,7 +67,7 @@ However, you can also create a repo on your local filesystem.

=== "Local Storage"

```python
```python exec="on" session="quickstart" source="above"
import icechunk
storage = icechunk.local_filesystem_storage("./icechunk-local")
Copy link
Contributor

Choose a reason for hiding this comment

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

How about

storage = icechunk.local_filesystem_storage(tempfile.mkdtemp()))

so we don't have to clear the local folder above?

@@ -14,7 +15,11 @@ build:
- poetry config virtualenvs.create false
post_install:
# Install deps and build using poetry
- . "$READTHEDOCS_VIRTUALENV_PATH/bin/activate" && cd docs && poetry install
- . "$READTHEDOCS_VIRTUALENV_PATH/bin/activate" && cd docs && poetry install && cd ../icechunk-python && maturin develop && cd ../docs
Copy link
Contributor

@dcherian dcherian Feb 25, 2025

Choose a reason for hiding this comment

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

we can directly install from github with pip too if we know the commit ID. example:

pip install git+https://github.com/earth-mover/icechunk.git@COMMIT#subdirectory=icechunk-python

This will require maturin in the env, which we seem to have.

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