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

Adding extra docs requirements and tweak conf #181

Closed
wants to merge 10 commits into from
Closed

Conversation

lsetiawan
Copy link
Contributor

Overview

This PR is my attempt in fixing the read the docs issue that's happening. I grabbed an example from https://github.com/django-auth-ldap/django-auth-ldap/blob/master/docs/conf.py

@weiji14
Copy link
Member

weiji14 commented Mar 11, 2021

What's the code for running the documentation build? Just wanted to test this out locally to see if it works. Edit: nevermind, figured it out:

cd icepyx/
pip install -r requirements-docs.txt
cd doc/
make html

@weiji14
Copy link
Member

weiji14 commented Mar 11, 2021

Can you try removing the import icepyx line. I don't think the icepyx package is used in this conf.py file. Edit: Hold on, might not be a good idea actually...

@weiji14
Copy link
Member

weiji14 commented Mar 11, 2021

I noticed that readthedocs.yaml can use method: setuptools (see https://docs.readthedocs.io/en/stable/config-file/v2.html#python). Wonder if that might do the trick?

diff --git a/readthedocs.yml b/readthedocs.yml
index b969696..f77e621 100644
--- a/readthedocs.yml
+++ b/readthedocs.yml
@@ -22,3 +22,4 @@ python:
   install:
     - requirements: requirements-docs.txt
     - requirements: requirements.txt
+  method: setuptools

@codecov-io
Copy link

Codecov Report

Merging #181 (cd26650) into development (08546a0) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##           development     #181   +/-   ##
============================================
  Coverage        48.17%   48.17%           
============================================
  Files               17       17           
  Lines             1179     1179           
  Branches           262      262           
============================================
  Hits               568      568           
  Misses             570      570           
  Partials            41       41           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 08546a0...cd26650. Read the comment docs.

@lsetiawan
Copy link
Contributor Author

I noticed that readthedocs.yaml can use method: setuptools (see https://docs.readthedocs.io/en/stable/config-file/v2.html#python). Wonder if that might do the trick?

diff --git a/readthedocs.yml b/readthedocs.yml
index b969696..f77e621 100644
--- a/readthedocs.yml
+++ b/readthedocs.yml
@@ -22,3 +22,4 @@ python:
   install:
     - requirements: requirements-docs.txt
     - requirements: requirements.txt
+  method: setuptools

I was wondering about that. Currently it's actually doing pip install ... it's strange that doesn't work since the pypi test works and that does a pip install ...

@JessicaS11
Copy link
Member

I noticed that readthedocs.yaml can use method: setuptools (see https://docs.readthedocs.io/en/stable/config-file/v2.html#python). Wonder if that might do the trick?

diff --git a/readthedocs.yml b/readthedocs.yml
index b969696..f77e621 100644
--- a/readthedocs.yml
+++ b/readthedocs.yml
@@ -22,3 +22,4 @@ python:
   install:
     - requirements: requirements-docs.txt
     - requirements: requirements.txt
+  method: setuptools

I was wondering about that. Currently it's actually doing pip install ... it's strange that doesn't work since the pypi test works and that does a pip install ...

I agree - odd indeed. I was able to get the docs to build on readthedocs by using this option (see #182). Thoughts or preferences for which approach we use?

@lsetiawan
Copy link
Contributor Author

lsetiawan commented Mar 11, 2021

I agree - odd indeed. I was able to get the docs to build on readthedocs by using this option (see #182). Thoughts or preferences for which approach we use?

Cool! If using setuptools work, then great. That's totally fine. We can close this PR in favor of #182. @JessicaS11 Could you bring in the doc workflow to your PR? Thanks! It would be good for us to test the doc build 😄

@lsetiawan lsetiawan closed this Mar 11, 2021
@JessicaS11
Copy link
Member

Could you bring in the doc workflow to your PR? Thanks! It would be good for us to test the doc build 😄

You and @weiji14 are on the same page (he just said the same thing in #170). I added it to #182 and am waiting for the checks to finish...

@JessicaS11
Copy link
Member

@lsetiawan Can I delete this branch?

@JessicaS11 JessicaS11 deleted the fix_docs branch April 18, 2023 19:20
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.

4 participants