updated dependencies needed for sphinx docs #261
Conversation
orbeckst
commented
Jul 3, 2023
orbeckst
commented
- fix building docs instructions miss dependencies #260
- updated required dependencies for sphinx docs (especially the bibtex packages)
- moved some packages from pip to conda
- fix #260 - updated required dependencies for sphinx docs (especially the bibtex packages) - moved some packages from pip to conda
doc/source/contributing_code.rst
Outdated
| @@ -217,11 +217,13 @@ First we need to install dependencies. You'll need a mix of conda and pip instal | |||
| griddataformats gsd hypothesis "joblib>=0.12" \ | |||
| matplotlib mmtf-python mock netcdf4 networkx \ | |||
| "numpy>=1.18.0" psutil pytest scikit-learn scipy \ | |||
There was a problem hiding this comment.
we probably should update the numpy minimum pin too
There was a problem hiding this comment.
yes, I just wanted to keep the commit focused
There was a problem hiding this comment.
will update to whatever I find in pyproject.toml
There was a problem hiding this comment.
If you want we can just merge this and I can just do a cleanup afterwards. There's probably a bunc of stuff in the page that needs changing.
There was a problem hiding this comment.
Just saw your comment after I did a bunch of things. If you think it's useful, merge and then we can update later.
There was a problem hiding this comment.
I think you've done more or less all the changes I think needed doing. I don't think we need two PRs. (it was mostly a "if you're kinda busy we can push it to the next PR")
doc/source/contributing_code.rst
Outdated
| 'pyedr>=0.7.0' \ | ||
| 'pytng>=0.2.3' \ | ||
| 'gsd>3.0.0' \ | ||
| 'rdkit>=2020.03.1' \ |
There was a problem hiding this comment.
not yet in pyproject.toml MDAnalysis/mdanalysis#4185
doc/source/contributing_code.rst
Outdated
| psutil pytest scikit-learn scipy "seaborn>=0.7.0" \ | ||
| sphinx sphinx_rtd_theme "tidynamics>=1.0.0" \ | ||
| "tqdm>=4.43.0" | ||
| 'Cython>=0.28' \ |
There was a problem hiding this comment.
It seems a bit superfluous to explicitly pip-install all of this, given that I can just do
pip install -E package/and then be assured to get the correct versions from pyproject.toml. Should we just recommend users do that?
This would also reduce the amount of text we have to keep updated.
There was a problem hiding this comment.
Yeah although we probably should do a pip install with all the extras?
There was a problem hiding this comment.
Yes, just forgot how to do it... was it something like
pip install .[extra-formats,analysis,doc]
There was a problem hiding this comment.
pip install ".[extra-formats,analysis,doc]" does the trick on my end
There was a problem hiding this comment.
I don't have time to rewrite the section and test that it all works. Just not sure if there's a good reason to split dependency installation and package building.
Sorry, going to punt here.
There was a problem hiding this comment.
Pretty sure it's fine to not split but we can deal with it later
| "numpy>=1.18.0" psutil pytest scikit-learn scipy \ | ||
| "seaborn>=0.7.0" sphinx "tidynamics>=1.0.0" \ | ||
| "tqdm>=4.43.0" | ||
| conda install -c conda-forge \ |
There was a problem hiding this comment.
I think it would be more convenient to use an environment.yaml file from the repo. Then we only need to keep that file up to date.
There was a problem hiding this comment.
I think there was a discussion about it on core at some point. The main thing was that we had a ton of different files lying around that didn't make sense (for example the RTD yaml file is pinned to different version numbers because of reasons I've not had a time to dig into yet).
What we could probably do is have a standard user facing one and have CI validate it (by picking up the setup-deps action in some way or another).
There was a problem hiding this comment.
Along the lines of "check if developer.yaml is compatible with the env that we are building via setup-deps"? Sounds like a good CI check.
Perhaps just build the env with setup-deps, then try to run mamba with the yaml file and check that nothing actually gets updated by diffing the env list before and after.
There was a problem hiding this comment.
yeah something like that should work
