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

Fix inclusion of random.js in HTML output #6935

Merged
merged 2 commits into from
May 15, 2023

Conversation

lagru
Copy link
Member

@lagru lagru commented May 13, 2023

Description

This script was removed in #6933 even though it is still in use on our front page (see scikit-image/skimage-web#85)! Therefore revert this particular change of #6933.

Checklist

  • Docstrings for all functions
  • Gallery example in ./doc/examples (new features only)
  • Benchmark in ./benchmarks, if your changes aren't covered by an
    existing benchmark
  • Unit tests
  • Clean style in the spirit of PEP8
  • Descriptive commit messages (see below)

For reviewers

  • Check that the PR title is short, concise, and will make sense 1 year
    later.
  • Check that new functions are imported in corresponding __init__.py.
  • Check that new features, API changes, and deprecations are mentioned in
    doc/release/release_dev.rst.
  • There is a bot to help automate backporting a PR to an older branch. For
    example, to backport to v0.19.x after merging, add the following in a PR
    comment: @meeseeksdev backport to v0.19.x
  • To run benchmarks on a PR, add the run-benchmark label. To rerun, the label
    can be removed and then added again. The benchmark output can be checked in
    the "Actions" tab.

This script was removed in scikit-image#6933 [1] even though it is still in use on our
front page [2]! Therefore revert this particular change of scikit-image#6933.

[1] scikit-image#6933
[2] scikit-image/skimage-web#85
@lagru lagru added 🤖 type: Infrastructure CI, packaging, tools and automation 🩹 type: Bug fix Fixes unexpected or incorrect behavior labels May 13, 2023
@lagru
Copy link
Member Author

lagru commented May 14, 2023

Hmm, random.js is actually missing from the artifacts generated by CircleCI. Though, it seems that has also been the case pre #6933. If it's a bug then it existed previously. I'll investigate tomorrow...

@lagru
Copy link
Member Author

lagru commented May 14, 2023

Wayback machine indicates that this broke between 2020-07-27 and 2020-08-15, though I can't find anything that looks like the cause in the commit history. @stefanv do you remember how the generated _static/random.js was supposed to be included in the corresponding folder in build/html?

Anyway I think I know why it isn't included. random.js is only created after sphinx is done

html: api
$(SPHINXBUILD) -b html $(ALLSPHINXOPTS) $(DEST)/html
@echo
@echo "Build finished. The HTML pages are in build/html."
$(PYTHON) source/random_gallery.py

There is nothing copying it to build/html/_static until sphinx is invoked again which doesn't happen in our build job.

Previously, `gallery_random.py` would generate the appropriate
`random.js` file with the `source/_static` as the target folder after
Sphinx itself was done. However, this meant that `random.js` would not
be included in our CI's build artifacts (locally it's included if
sphinx is run a second time).

Inclusion in skimage_extensions.py with a trigger on the Sphinx event
"build-finished" ensures that the appropriate output path can be
determined from Sphinx's configuration.

This inclusion seems to have been broken at least since 2020-08-15 [2].

[2] http://web.archive.org/web/20200815074104/https://scikit-image.org/
@lagru
Copy link
Member Author

lagru commented May 14, 2023

Okay, it's included now (see artifacts).

@lagru lagru changed the title Revert removal of random gallery script Fix inclusion of random.js in HTML output May 14, 2023
@jarrodmillman jarrodmillman added this to the 0.21 milestone May 15, 2023
@jarrodmillman jarrodmillman merged commit 4d6d1a0 into scikit-image:main May 15, 2023
20 checks passed
@lagru lagru deleted the readd-random-gallery branch May 16, 2023 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🩹 type: Bug fix Fixes unexpected or incorrect behavior 🤖 type: Infrastructure CI, packaging, tools and automation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants