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

readthedocs: Add search ranking and use latest Python version #242

Merged
merged 9 commits into from Apr 24, 2023

Conversation

EwoutH
Copy link
Collaborator

@EwoutH EwoutH commented Apr 3, 2023

Add a ranking which search results should pop up first (proposed docs/source > ema_workbench/examples > ema_workbench > everything else > all models).

This will make sure that the docs pop up on top, then the examples, then the source code, then all other stuff (including tests) and then the included models. Please let know what you think of this ranking.

Some files are also excluded, most notably the data folders.

See Configuration file v2#search

Also always use latest stable Python version and build a downloadable htmlzip.

Furthermore, we could consider adding Search as you type with the readthedocs-sphinx-search extension. Also we might take a look (during the course) on search analytics to see where we can improve.

@EwoutH EwoutH added the docs label Apr 3, 2023
@EwoutH EwoutH added this to the 2.4.0 milestone Apr 3, 2023
@EwoutH EwoutH requested a review from quaquel April 3, 2023 15:18
Add a ranking which search results should pop up first (proposed docs/source > ema_workbench/examples > ema_workbench > everything else > all models).

See https://docs.readthedocs.io/en/stable/config-file/v2.html#search

Also always use latest stable Python version.
@EwoutH EwoutH force-pushed the readthedocs-search-ranking branch from 0acdc4b to 506ff0f Compare April 3, 2023 15:21
Copy link
Owner

@quaquel quaquel left a comment

Choose a reason for hiding this comment

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

Looks good and ranking makes sense.

@EwoutH
Copy link
Collaborator Author

EwoutH commented Apr 3, 2023

Thanks!

For me the searching is stuck, both on this PR and the latest stable docs. Does search work for you?

3A7AE1D7-DE62-4848-AEE3-3BBEF5BB26BA

@quaquel
Copy link
Owner

quaquel commented Apr 3, 2023

for me as well. Seems related to this StackOverflow issue

edit: checked readthedocs and indeed it is this exact issue. So all that is needed is forcing the use of the latest version of the theme.

edit2: the fix seems to be to include sphinx_rtd_theme>0.5 explicitly in pyproject.toml.

@EwoutH
Copy link
Collaborator Author

EwoutH commented Apr 3, 2023

Good catch!

edit2: the fix seems to be to include sphinx_rtd_theme>0.5 explicitly in pyproject.toml.

And this again seemed to be caused by readthedocs/readthedocs.org#7858.

If we look at a recent build log, we're currently installing:

Collecting sphinx<2
  Downloading Sphinx-1.8.6-py2.py3-none-any.whl (3.1 MB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 3.1/3.1 MB 150.6 MB/s eta 0:00:00
Collecting sphinx-rtd-theme<0.5
  Downloading sphinx_rtd_theme-0.4.3-py2.py3-none-any.whl (6.4 MB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 6.4/6.4 MB 179.6 MB/s eta 0:00:00
Collecting readthedocs-sphinx-ext<2.3
  Downloading readthedocs_sphinx_ext-2.2.0-py2.py3-none-any.whl (11 kB)

But then in the next step, where our optional docs dependencies get installed, this is overwritten:

Requirement already satisfied: sphinx in /home/docs/checkouts/readthedocs.org/user_builds/emaworkbench/envs/latest/lib/python3.10/site-packages (from ema-workbench==2.4.0.dev0) (1.8.6)
Collecting sphinx
  Downloading sphinx-6.1.3-py3-none-any.whl (3.0 MB)

So we get sphinx>=2 and sphinx-rtd-theme<0.5, which aren't compatible with each other.

Let me try to fix that.

- Set sphinx-rtd-theme as optional docs dependency
- Specify needs_sphinx and needs_extensions in the conf.py configuration file (see https://www.sphinx-doc.org/en/master/usage/configuration.html#confval-needs_sphinx)
@EwoutH EwoutH force-pushed the readthedocs-search-ranking branch from 02ae337 to 9baad82 Compare April 3, 2023 18:00
@EwoutH
Copy link
Collaborator Author

EwoutH commented Apr 3, 2023

Of course nothing can't be simple.

I will take a better look at this later this week.

@coveralls
Copy link

coveralls commented Apr 9, 2023

Coverage Status

Coverage: 81.097% (-0.1%) from 81.227% when pulling 5433e08 on readthedocs-search-ranking into 081b41f on master.

@quaquel
Copy link
Owner

quaquel commented Apr 9, 2023

I checked the build LOG and we are now installing the correct versions. However, this does not seem to resolve the problem. Reading up on readthedocs, they seem to still prefer using requirements files instead of pyproject.toml. See https://docs.readthedocs.io/en/stable/guides/reproducible-builds.html. So, we might give that a try by having a requirements.in and requirements.txt in the docs directory somewhere. See also https://github.com/readthedocs/readthedocs.org/tree/main/requirements for some examples.

@EwoutH
Copy link
Collaborator Author

EwoutH commented Apr 9, 2023

Yeah I also noticed that. Let's see what they think of it upstream: readthedocs/readthedocs.org#9642 (comment)

@quaquel quaquel modified the milestones: 2.4.0, 2.5.0 Apr 19, 2023
@EwoutH
Copy link
Collaborator Author

EwoutH commented Apr 21, 2023

Tried following their documentation to the letter, but still doesn't work. This is one of the most stubborn issues I encountered in a long time.

I filed a new issue upstream: readthedocs/readthedocs.org#10263

@quaquel quaquel merged commit 0ae17f5 into master Apr 24, 2023
10 of 11 checks passed
@EwoutH
Copy link
Collaborator Author

EwoutH commented Apr 24, 2023

I would have liked to cleaned up this PR some more, and test the search ranking some more. Please wait for explicit go ahead for merging, especially after such a convoluted history.

Anyway, glad to have search working again, I will test tomorrow if the search ordering makes sense, and probably open a PR to revert the docs requirements changes.

@quaquel
Copy link
Owner

quaquel commented Apr 24, 2023

My mistake. Had github open, saw the e-mail about the search fix, and just clicked merge on the only open pull request without first refreshing Github.

EwoutH added a commit that referenced this pull request Jun 18, 2023
* readthedocs: Add search ranking and use latest Python version

Add a ranking which search results should pop up first (proposed docs/source > ema_workbench/examples > ema_workbench > everything else > all models).

See https://docs.readthedocs.io/en/stable/config-file/v2.html#search

Also always use latest stable Python version.

* docs: sphinx-rtd-theme as optional dep, specify needs_sphinx

- Set sphinx-rtd-theme as optional docs dependency
- Specify needs_sphinx and needs_extensions in the conf.py configuration file (see https://www.sphinx-doc.org/en/master/usage/configuration.html#confval-needs_sphinx)

* Update .readthedocs.yaml

Try something else to set the docs requirements

* Update .readthedocs.yaml

* readthedocs-sphinx-ext update

* Attempt to fix search by using requirements.txt

* Add general requirements to docs

* docs: conf.py: Implement search workaround

Implement a workaround to get search working again

readthedocs/sphinx_rtd_theme#1452 (comment)

---------

Co-authored-by: Jan Kwakkel <j.h.kwakkel@tudelft.nl>
@quaquel quaquel deleted the readthedocs-search-ranking branch November 18, 2023 10:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants