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

Initial documentation setup with mkdocs #173

Closed
wants to merge 7 commits into from

Conversation

melissawm
Copy link
Member

@melissawm melissawm commented May 2, 2023

Here is an attempt at fixing #70. This is not a jupyterbook setup, but a plain mkdocs setup that also uses mkdocs-gallery.

I am still trying to figure out the correct way to execute gallery examples. As of now, they will run but the napari viewer window will not close by itself, which prevents correct execution of the gallery. qtgallery still doesn't work correctly but I'm working on it.

@alisterburt
Copy link
Collaborator

great stuff @melissawm - please ping when you think this is ready! Alternatively I'm happy to give you write access to the repo for docs work, let me know what you prefer

@psobolewskiPhD
Copy link
Member

@melissawm Do you think we can use the same fix as in the napari/docs repo?
napari/docs#207

def napari_scraper(block, block_vars, gallery_conf):
    """Basic napari window scraper.

    Looks for any QtMainWindow instances and takes a screenshot of them.

    `app.processEvents()` allows Qt events to propagateo and prevents hanging.
    """
    imgpath_iter = block_vars['image_path_iterator']

    if app := napari.qt.get_app():
        app.processEvents()
    else:
        return ""

    img_paths = []
    for win, img_path in zip(
        reversed(napari._qt.qt_main_window._QtMainWindow._instances),
        imgpath_iter,
    ):
        img_paths.append(img_path)
        win._window.screenshot(img_path, canvas_only=False)

    napari.Viewer.close_all()
    app.processEvents()

    return scrapers.figure_rst(img_paths, gallery_conf['src_dir'])

@melissawm
Copy link
Member Author

I'll try!

Also updates README and adds title to new example
@melissawm
Copy link
Member Author

I think I've made progress with the scraper but now I'm hitting #181, so I'll wait for that to go in and will update accordingly. Cheers!

@melissawm melissawm marked this pull request as draft November 27, 2023 17:27
@psobolewskiPhD
Copy link
Member

@melissawm #181 is merged! Let me know if I can help get this one over the line!
❤️

@Czaki
Copy link
Contributor

Czaki commented Dec 14, 2023

Why mkadocs? I know that it is simpler, but as it is napari project then may be nice to use the same theme.

@psobolewskiPhD
Copy link
Member

Why mkadocs? I know that it is simpler, but as it is napari project then may be nice to use the same theme.

Good point. I do think that mkdocs was @alisterburt preference, see:
#70 (comment)

@melissawm
Copy link
Member Author

I'm going to get this in shape, but if the preference is to use the new napari theme I am happy to come up with an alternative version ☺️

@psobolewskiPhD
Copy link
Member

@melissawm I certainly don't want to throw away your efforts with this PR! But beyond the theme, I think using the same docs tooling (sphinx & myst) as for napari docs would make maintenance easier?
What do you think?

As an aside, I think this plugin has such unique napari value that it should become part of "napari[all]" so consistent theme does make sense in that context.

Fix CI check failure by ordering import statements with isort.
isort formatting import statements - fix for CI check
@GenevieveBuckley
Copy link
Contributor

@psobolewskiPhD

I certainly don't want to throw away your efforts with this PR! But beyond the theme, I think using the same docs tooling (sphinx & myst) as for napari docs would make maintenance easier?
What do you think?

As an aside, I think this plugin has such unique napari value that it should become part of "napari[all]" so consistent theme does make sense in that context.

I'm +1 on the idea that napari-animation should be included with "napari[all]".

I think it might be nice to have the same website theme as core napari, but I do not think it is worth spending developer time on if this is going to be annoying. That could be a future PR, but I don't think it's important.

Previously, @alisterburt expressed a strong preference for using the mkdocs-material theme, and staying away from sphinx (here and here). I agree with Alister, I think a sphinx/myst setup would be much more complicated to maintain than mkdcos-gallery.

It is much more important to me personally that we get the functional benefits of good documentation (i.e. the info and gallery examples are readable on the website). I do not care about branding & whether the website uses the same shade of blue, font, or theme, etc. At this stage of the project, it's better to spend that developer time on functionality (eg: fixing bugs, or generating more/better content for documentation, etc.)

@GenevieveBuckley
Copy link
Contributor

This looks close, so I'd like for us to try and get this across the finish line @melissawm. Are there any blockers remaining from your point of view?

The only big thing I think is to make sure github actions updates the docs and gallery examples when needed.

I've previously done something similar here for the napari-threedee project. We update the github actions yaml, and make sure we have secrets.DOCS_DEPLOY_TOKEN set up for the repository, eg:

    # Build the book
    - name: Build the book
      run: |
        mkdocs build
    # Push the book's HTML to github-pages whenever the main repo branch changes
    - name: GitHub Pages action
      if: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }}
      uses: peaceiris/[email protected]
      with:
        deploy_key: ${{ secrets.DOCS_DEPLOY_TOKEN }}
        external_repository: napari-animation/napari-animation.github.io
        publish_dir: ./site

@GenevieveBuckley
Copy link
Contributor

I will say that when I run mkdocs serve, the gallery examples output static images, rather than the movies shown here https://napari-animation-docs.readthedocs.io/en/latest/ (like this).

Having actual animations would be ideal for the napari-animation docs eventually, but I think the static 2D images are a big step forward regardless (essentially they are an infinite improvement all on their own, because otherwise new users would never even know from the docs that examples exist at all).

@psobolewskiPhD
Copy link
Member

psobolewskiPhD commented Jan 15, 2024

Ironically that site ( https://github.com/kolibril13/napari-animation-docs/blob/main/docs/source/conf.py ) uses sphinx.
Anyhow I think the key to the animations showing is the helper function, see:
https://github.com/kolibril13/napari-animation-docs/blob/main/docs/source/notebook.ipynb
We could try something similar as a hidden cell showing just output?
Edit: actually I think it would require changing the scraper...

BTW it doesn't look like on that site that the examples are run on CI, but rather locally and the videos are embedded in the notebooks. So a bit different case I think, but maybe would work

@melissawm
Copy link
Member Author

Yup, I was waiting on a decision regarding the sphinx vs. mkdocs discussion to then figure out the actual animations. I guess I didn't want to spend time on one stack if we're going with the other in the end 😅

@jni
Copy link
Member

jni commented Jan 15, 2024

fwiw my leaning is the opposite to Alister's. You make good points @GenevieveBuckley about "whatever gets us going", and I definitely agree, but that may or may not be what helps us maintain the documentation in the long term — having two stacks is not going to be helpful re long term maintenance here. (I do agree very much about theming being a low priority.)

imho mkdocs filled a niche three years ago when Alister made those comments of allowing easy authoring with markdown, which was not easy at all in sphinx. Today myst and myst-nb have got that use case well covered, so I don't think the pain points with sphinx are what they used to be, and there is lots of value in using the same toolchain for all our projects.

Additionally, if we get automatic movies going with napari-animation in mkdocs it may be additional work to display automatic movies in napari docs proper, whereas the work can get used by multiple napari projects if it happens in sphinx-land.

@melissawm
Copy link
Member Author

Ok - I am happy to set up an alternate version with sphinx, it should be simple enough to get up and running quickly. If that makes the animations easier, at least we'll have that data point for the decision. Does that sound reasonable?

@jni
Copy link
Member

jni commented Jan 16, 2024

imho yes! Thank you @melissawm!

@GenevieveBuckley
Copy link
Contributor

GenevieveBuckley commented Jan 24, 2024

I've built working video scrapers for both mkdocs and sphinx setups now. They work fine on my local projects, so I guess the question is do we have a sphinx setup here (assuming the mkdocs plan is overruled) that I can slot them into.

@melissawm
Copy link
Member Author

Let me do that today - I'll push a different pr and then we can check. I'll ping you when ready

@GenevieveBuckley
Copy link
Contributor

Let me do that today - I'll push a different pr and then we can check. I'll ping you when ready

Ok, sure. Tomorrow is a public holiday, so I can either work on it today or get to it next week.

GenevieveBuckley added a commit that referenced this pull request Feb 2, 2024
Supersedes #173 
May close #80 
@melissawm implements the sphinx documentation setup, similar to the napari/docs repository documentation setup
@GenevieveBuckley adds scrapers for the examples gallery (for videos and static napari screenshots), and adds the github actions docs deployment
@GenevieveBuckley
Copy link
Contributor

Superseded by #200

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.

6 participants