Update conf.py, sphinx context injection deprecated in ReadTheDocs

[GenevieveBuckley]

Closes #382

ReadTheDocs is deprecating sphinx context injection. We need to set the canonical url a different way.

[GenevieveBuckley]

Right now the code change to conf.py is a straight copy-paste from the suggested code snippet.

Questions:

• Are there any sphinx extensions in a dependency list somewhere that we need to remove?
• Should I change this line to not use an empty string as the default, but instead show image.dask.org, like this?

html_baseurl = os.environ.get("READTHEDOCS_CANONICAL_URL", "image.dask.org")

[m-albert]

Don't have any input here for now but I'm linking dask/dask#11229.

[GenevieveBuckley]

Better now? https://dask-image--383.org.readthedocs.build/en/383/api.html

[jakirkham]

Oops sorry missed your comment Genevieve 😞

Given it passes RTD, yes! 🎉

[jakirkham]

rerun tests

[jakirkham]

• Are there any sphinx extensions in a dependency list somewhere that we need to remove?

There is this list of dependencies

dask-image/continuous_integration/environment-doc.yml

Lines 20 to 30 in 77a29ba

	# FIXME: This workaround is required until we have sphinx>=5, as enabled by
	# dask-sphinx-theme no longer pinning sphinx-book-theme==0.2.0. This is
	# tracked in https://github.com/dask/dask-sphinx-theme/issues/68.
	# Once sphinx>=5 is available, we can remove this workaround.
	- dask-sphinx-theme>=3.0.0
	- sphinx>=4.0.0
	- sphinxcontrib-applehelp>=1.0.0,<1.0.7
	- sphinxcontrib-devhelp>=1.0.0,<1.0.6
	- sphinxcontrib-htmlhelp>=2.0.0,<2.0.5
	- sphinxcontrib-serializinghtml>=1.1.0,<1.1.10
	- sphinxcontrib-qthelp>=1.0.0,<1.0.7

Also this list of Sphinx extensions

dask-image/docs/conf.py

Lines 41 to 48 in 77a29ba

	# Add any Sphinx extension module names here, as strings. They can be
	# extensions coming with Sphinx (named 'sphinx.ext.*') or your custom ones.
	extensions = [
	'sphinx.ext.autodoc',
	'sphinx.ext.viewcode',
	'sphinx.ext.todo',
	'sphinx.ext.napoleon'
	]

Perhaps we want to revisit the former (later). That looks unrelated

The latter all come from Sphinx. So think we can leave them as-is

The blogpost also mentions some new features like flyout_display and addons

Personally am more interested in keeping the existing docs functional than adding new bells and whistles, but could be convinced otherwise if there are compelling needs/interests in enabling these

[jakirkham]

Thanks Genevieve! 🙏

After reading the blogpost more closely, realize I goofed in my last suggestion. Included a new one based on my re-reading. Though please double check

Also tried to answer your other questions. Please let me know whether that is helpful. Happy to discuss anything further as needed

Thanks again for picking this up 🙂

[jakirkham]

• Should I change this line to not use an empty string as the default, but instead show image.dask.org, like this?

html_baseurl = os.environ.get("READTHEDOCS_CANONICAL_URL", "image.dask.org")

After reading this doc page, it sounds like RTD checks the domains listed in the RTD settings, which is just image.dask.org (as seen below)

Then passes it in as the environment variable READTHEDOCS_CANONICAL_URL, which we pick up on the code line above

While we could change the fallback default, it is sounds like

1. Sounds like RTD does the right for us
2. Having a default that smokes out any issues (like empty string does) is useful
3. DRY

Think for these reasons having the setting in one place should work for us and keep maintenance straightforward

Though do appreciate this question and the exercise of thinking through it was helpful

[jakirkham]

Pushed a small fix for html_context based on this RTD blogpost

[jakirkham]

Thanks Genevieve! 🙏

Approving as I think this is all we need. Though please double check

If we do find something needs a change post-merge, we can always do another PR

[GenevieveBuckley]

Thanks John!