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

Update RTD config #128

Merged
merged 4 commits into from
Aug 9, 2023
Merged

Update RTD config #128

merged 4 commits into from
Aug 9, 2023

Conversation

dwhswenson
Copy link
Owner

Apparently docs were not building correctly, and furthermore, we had some setting to use system packages, which will no longer be allowed.

Let's get that fixed.

Apparently docs were not building correctly, and furthermore, we had
some setting to use system packages, which will no longer be allowed.

Let's get that fixed.
@codecov
Copy link

codecov bot commented Aug 8, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (3ae38ea) 100.00% compared to head (f01af32) 100.00%.

Additional details and impacted files
@@            Coverage Diff            @@
##            master      #128   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           13        13           
  Lines         1154      1154           
=========================================
  Hits          1154      1154           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@sroet sroet left a comment

Choose a reason for hiding this comment

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

1 unnecessary line seems to be added, otherwise LGTM

.readthedocs.yml Show resolved Hide resolved
@dwhswenson
Copy link
Owner Author

NB: this is still failing in RTD, so it needs some more work. @sroet : I noticed that you aren't listed as a maintainer with RTD; an oversight because I so rarely need to log into the RTD interface. Do you use the same username over there?

see if we can also get this to override sphinx defaults
@sroet
Copy link
Collaborator

sroet commented Aug 9, 2023

Do you use the same username over there?

I do

Copy link
Collaborator

@sroet sroet left a comment

Choose a reason for hiding this comment

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

LGTM, feel free to merge if this works on RTD

@sroet
Copy link
Collaborator

sroet commented Aug 9, 2023

our builds seem to hit sphinx-contrib/napoleon#9 which became breaking in python 3.10+ not sure why that is still an issue when it became packaged with sphinx

EDIT: it seems like RTD is pulling in sphinx<2 in the line

python -m pip install --upgrade --no-cache-dir pillow mock==1.0.1 alabaster>=0.7,<0.8,!=0.7.5 commonmark==0.9.1 recommonmark==0.5.0 sphinx<2 sphinx-rtd-theme<0.5 readthedocs-sphinx-ext<2.3 jinja2<3.1.0

I think for now the easiest solution is to run python 3.9 for the doc builds

@sroet sroet self-requested a review August 9, 2023 16:56
Copy link
Collaborator

@sroet sroet left a comment

Choose a reason for hiding this comment

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

Seems like that build passed, feel free to merge!

@dwhswenson
Copy link
Owner Author

Yep, looks like 3.9 was enough!

The sphinx<2 is because older projects have some ancient defaults. (I think we started in October 2017). I think we may need to eventually update docs/rtd_requirements.txt to ask for more recent versions, but this works for now!

@dwhswenson dwhswenson merged commit 7fbb0b0 into master Aug 9, 2023
8 checks passed
@dwhswenson dwhswenson deleted the update-rtd branch August 9, 2023 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants