Skip to content

Moving docs' requirements into the actual docs' requirements file#72

Merged
4 commits merged intomainfrom
maa/fix-docs-build
Feb 10, 2023
Merged

Moving docs' requirements into the actual docs' requirements file#72
4 commits merged intomainfrom
maa/fix-docs-build

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Feb 9, 2023

Reference Issue or PRs

This PR is not linked to an existing issue.

What does your PR implement? Be specific.

This PR moves the specific requirements of the documentation (matplotlib and gitpython) which are currently in the setup.py file to the docs/requirements.txt file, which is more appropriate as they are not used elsewhere.

@ghost ghost requested a review from maikia February 9, 2023 08:37
Copy link
Copy Markdown
Collaborator

@maikia maikia left a comment

Choose a reason for hiding this comment

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

@mandreux-owkin thanks for your contribution!
the cleaning up is a great idea, and it's true that gitpython and matplotlib should not be in setup.py if they are only used for docs.
but here it might be a bit more complex:
Sphinx is generating the docs, but we also use binder to run the example notebooks. (refer to .mybinder directory) There, in postbuild we are actually uninstalling all the docs/requirements to have a lighter build. I am afraid with this change the binder won't work. Feel free to suggest alternative solution.

@ghost
Copy link
Copy Markdown
Author

ghost commented Feb 10, 2023

@maikia thanks for your careful review. I checked the notebooks code and I am certain that gitpython is not used. We don't use any plot in the jupyter but I added matplotlib again to avoid breaking binder

Copy link
Copy Markdown
Collaborator

@maikia maikia left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @mandreux-owkin

@ghost ghost merged commit 37ef603 into main Feb 10, 2023
@ghost ghost deleted the maa/fix-docs-build branch February 10, 2023 13:51
This pull request was closed.
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