-
Notifications
You must be signed in to change notification settings - Fork 36
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
fix installation instructions and recommend mamba instead of conda #370
Conversation
contributions to the PR are welcome! |
@orbeckst I might either take over or overwrite this PR later today - I had started working on my own update earlier. |
By all means. |
Where are you with the update that you mentioned @IAlibay ? |
@yuxuanzhuang if you want to do a quick minimal update to the instructions then we can at least get a quick-fix into the user guide. This PR already introduces mamba; you could add your changes to this PR. |
doc/source/contributing_code.rst
Outdated
@@ -243,7 +249,7 @@ First we need to install dependencies. You'll need a mix of conda and pip instal | |||
'mda-xdrlib' | |||
|
|||
# documentation dependencies | |||
conda install -c conda-forge 'mdanalysis-sphinx-theme>=1.3.0' docutils sphinx-sitemap sphinxcontrib-bibtex pybtex pybtex-docutils | |||
mamba install -c conda-forge 'mdanalysis-sphinx-theme>=1.3.0' docutils sphinx-sitemap sphinxcontrib-bibtex pybtex pybtex-docutils | |||
python -m pip install docutils sphinx-sitemap sphinxcontrib-bibtex pybtex pybtex-docutils |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason these deps are installed both with mamba and pip here?
Sorry I'm travelling / handling work things - I'll be free to do MDAnalysis things, including this, over the weekend. |
4e76622
to
e8e4d6d
Compare
@yuxuanzhuang @RMeli we do need this PR to be merged so that we have some instructions for the hackathon. (Ideally we also get a release of the User Guide but that's probably a different thing.) |
doc/source/contributing_code.rst
Outdated
an isolated environment without touching your base Python installation, and without | ||
touching existing environments that may have stable versions of MDAnalysis. : | ||
|
||
.. code-block:: bash | ||
|
||
conda create --name mdanalysis-dev "python>=3.9" | ||
mamba create --name mdanalysis-dev "python>=3.10" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we recommend the latest supported Python here instead? We will have three version changes before the docs going out of date. ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. Can I ask you to try out the installation instructions below for the version you want to put here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sorry but I'm AFK until Friday morning (UTC+2). Please discard my comment if this needs to be merged quickly, it was just a "nice to have" suggestion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tried it with 3.12 and it works, added 3.12
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
I assume we should update the UserGuide again once the parallel analysis PR is merged?
doc/source/contributing_code.rst
Outdated
You can also install the following optional dependencies, although please note | ||
that they many not all be available for your machine type. Specifically; | ||
hole2, and distopia are only available on linux + x86_64 machines, and openmm | ||
is not available for windows. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add "Remove the packages listed as incompatible from the command below and run it again."
Alternatively, we can show example code for different os with sphinxcontrib.osexample
(https://sublime-and-sphinx-guide.readthedocs.io/en/latest/code_blocks.html)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added something similar
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sphinxcontrib.osexample
as used in https://sublime-and-sphinx-guide.readthedocs.io/en/latest/code_blocks.html#code-examples-in-multiple-languages looks like a neat addition. Maybe open new issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested the installation instructions for Python 3.12 on my x86_64 macOS and was able to install everything via mamba except clustalw.
And all tests pass
so I am going to merge. Thanks for the reviews & contributions! |
📚 Documentation preview 📚: https://mdanalysisuserguide--370.org.readthedocs.build/en/370/