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

Add sphinx_rtd_theme to docs dependencies #301

Merged
merged 10 commits into from
Sep 12, 2023
Merged

Conversation

will-moore
Copy link
Member

Try to fix sphinx readthedocs build - similar to:
ome/omero-scripts@07e130d

@will-moore
Copy link
Member Author

@jburel Thanks for the pointer. Not sure if I've applied it wrong (I have docs/source/conf.py instead of docs/conf.py) or if that error needs a different fix?

@jburel
Copy link
Member

jburel commented Aug 25, 2023

you will need to declare it in the .readthedocs.yml file
i.e.

python:
  install:
    - requirements: docs/source/requirements.txt

@jburel
Copy link
Member

jburel commented Aug 25, 2023

It did not install it

@will-moore
Copy link
Member Author

@jburel The error you had at ome/omero-scripts#187
https://github.com/ome/omero-scripts/actions/runs/5925626167/job/16065416934
was

Extension error:
Could not import extension sphinx_rtd_theme (exception: No module named 'sphinx_rtd_theme')

whereas I'm seeing

Traceback (most recent call last):
  File "/home/docs/checkouts/readthedocs.org/user_builds/ome-zarr/envs/301/lib/python3.9/site-packages/sphinx/config.py", line 350, in eval_config_file
    exec(code, namespace)
  File "/home/docs/checkouts/readthedocs.org/user_builds/ome-zarr/checkouts/301/docs/source/conf.py", line 85, in <module>
    'html_theme': html_theme,
NameError: name 'html_theme' is not defined

at https://readthedocs.org/projects/ome-zarr/builds/21717507/

Also noticed that one of those jobs is github actions, the other is on readthedocs...

@@ -1,2 +1,2 @@
sphinx==5.3.0
sphinx_rtd_theme==1.1.1
Copy link
Member

@jburel jburel Aug 25, 2023

Choose a reason for hiding this comment

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

I will avoid using an rc
you don't need it here

@will-moore
Copy link
Member Author

Hah, got there in the end!
Please merge if OK as this is failing on all PRs just now

@will-moore
Copy link
Member Author

@joshmoore / @jburel OK to merge?

@@ -0,0 +1,7 @@
sphinx==7.1.2
sphinx-rtd-theme==1.3.0rc1
Copy link
Member

Choose a reason for hiding this comment

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

Why did you pick the rc?

Copy link
Member Author

Choose a reason for hiding this comment

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

See 492b29c - I copied a template (link in the commit)

Copy link
Member

Choose a reason for hiding this comment

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

but you don't need the rc to get it to work

Copy link
Member Author

Choose a reason for hiding this comment

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

I wasn't taking any chances!
But OK, removed the rc now.

@will-moore
Copy link
Member Author

@jburel - Green again - good to merge (since other PRs have failing builds just now)?

@jburel
Copy link
Member

jburel commented Sep 12, 2023

Thanks

@jburel jburel merged commit f9e5dfc into ome:master Sep 12, 2023
12 checks passed
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