Skip to content

Improve conda installation instructions#2246

Merged
rwest merged 3 commits into
mainfrom
mef_install
May 26, 2023
Merged

Improve conda installation instructions#2246
rwest merged 3 commits into
mainfrom
mef_install

Conversation

@mefuller

Copy link
Copy Markdown
Member

Motivation or Problem

Writing the documentation to only reference Anaconda is inflexible and bloated as compared with utilizing Miniconda or Conda available in system repositories. This is of particular relevance of storage-limited systems, such as shared high-performance resources where RMG is often installed (which also frequently run RHEL or another Enterpise Linux which includes conda in the repositories, easing administrative burden).

Description of Changes

  • Use repositories over Ananconda for proper locations, updates
  • switch to Miniconda to reduce download and installation size by 10x
  • replace deprecated source with conda in activating environments

Testing

Environments created and RMG-Py, RMG-database installed and tested on Fedora and EL systems

@sevyharris sevyharris left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks good to me. I think it should be ready to go after the two tabs are added so the HTML builds properly.

Comment thread documentation/source/users/rmg/installation/anacondaUser.rst Outdated
Comment thread documentation/source/users/rmg/installation/anacondaDeveloper.rst Outdated
@JacksonBurns

Copy link
Copy Markdown
Contributor

Something like this came up in #2437 - the Docker image and CI use miniconda, which does not follow the developer installation instructions. For the same reasons listed in this PR, this should be changed. I am going to resolve the merge conflicts and get this ready to merge, since the same points made are still relevant. @sevyharris could you review after?

@JacksonBurns

Copy link
Copy Markdown
Contributor

Also adding that almost all of the content that was originally in this PR (like conda activate instead of source activate) has trickled in over time via other PRs.

@mefuller

Copy link
Copy Markdown
Member Author

So you want me to tidy it up?

@JacksonBurns

Copy link
Copy Markdown
Contributor

So you want me to tidy it up?

Nope, there was only one conlict, which I resolved. The rest of the changes were already present, so they did not raise conflicts when updating. Thanks for checking in though!

@codecov

codecov Bot commented May 23, 2023

Copy link
Copy Markdown

Codecov Report

Merging #2246 (353d769) into main (770a8c8) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #2246   +/-   ##
=======================================
  Coverage   48.13%   48.13%           
=======================================
  Files         110      110           
  Lines       30626    30626           
  Branches     7988     7988           
=======================================
  Hits        14743    14743           
  Misses      14353    14353           
  Partials     1530     1530           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@rwest

rwest commented May 24, 2023

Copy link
Copy Markdown
Member

I fixed the issues Sevy noted, and added some more edits.
Looks good to me, but as many of the changes are now mine, we should have someone else review it too.
(fwiw I did build the documentation locally, which for me required some hacks described here, and it rendered OK in HTML)

mefuller and others added 3 commits May 26, 2023 11:22
…nda for proper locations, updates; switch to Miniconda to reduce download and installation size by 10x; replace deprecated "source" with "conda"
Whitespace is crucial, a bit like in Python.
One needs to actually compile in sphinx and look at the output to be sure.
Make a sub-list for different package managers.
Tweak the MacOS instructions wording.
@rwest rwest enabled auto-merge May 26, 2023 19:00
@rwest rwest requested a review from JacksonBurns May 26, 2023 19:00

@JacksonBurns JacksonBurns left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@rwest rwest merged commit 40b5558 into main May 26, 2023
@rwest rwest deleted the mef_install branch May 26, 2023 23:26
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.

4 participants