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

Change workflow to mamba #429

Merged
merged 3 commits into from
Jun 14, 2023
Merged

Change workflow to mamba #429

merged 3 commits into from
Jun 14, 2023

Conversation

forsyth2
Copy link
Collaborator

Change workflow to mamba. Resolves #397.

@forsyth2 forsyth2 added the DevOps CI/CD, configuration, etc. label May 24, 2023
@forsyth2 forsyth2 self-assigned this May 24, 2023
@forsyth2
Copy link
Collaborator Author

@chengzhuzhang Once this pull request and #428 merged, I can probably create equivalent pull requests for both zstash and E3SM Diags.

@tomvothecoder I believe in our meeting, we discussed that the only necessary change was to remove the intall_requires=["configobj>=5.0.0,<6.0.0", "jinja2>=2.0.0"], line. I have removed that here.

git grep -n pip shows a number of matches. So we're ok with keeping these instances?

.github/workflows/build_workflow.yml:81:        run: pip install .
.github/workflows/build_workflow.yml:106:      - name: Cache pip
.github/workflows/build_workflow.yml:110:          path: ~/.cache/pip
.github/workflows/build_workflow.yml:112:          key: ${{ runner.os }}-pip-publish-docs
.github/workflows/build_workflow.yml:114:            ${{ runner.os }}-pip-
.github/workflows/build_workflow.yml:117:      # Using pip for Sphinx dependencies because it takes too long to reproduce a conda environment (~10 secs vs. 3-4 mins)
.github/workflows/build_workflow.yml:120:          python -m pip install --upgrade pip
.github/workflows/build_workflow.yml:121:          pip install sphinx==5.2.3 sphinx_rtd_theme==1.0.0 docutils==0.16
.github/workflows/release_workflow.yml:26:      - name: Cache pip
.github/workflows/release_workflow.yml:30:          path: ~/.cache/pip
.github/workflows/release_workflow.yml:32:          key: ${{ runner.os }}-pip-publish-docs
.github/workflows/release_workflow.yml:34:            ${{ runner.os }}-pip-
.github/workflows/release_workflow.yml:37:      # Using pip for Sphinx dependencies because it takes too long to reproduce a conda environment (~10 secs vs. 3-4 mins)
.github/workflows/release_workflow.yml:40:          python -m pip install --upgrade pip
.github/workflows/release_workflow.yml:41:          pip install sphinx==5.2.3 sphinx_rtd_theme==1.0.0 docutils==0.16
conda/dev.yml:9:  - pip=22.2.2
conda/dev.yml:33:  - pip:
conda/meta.yaml:14:  script: "{{ PYTHON }} -m pip install . --no-deps -vv"
conda/meta.yaml:22:    - pip
conda/meta.yaml:32:    - pip
conda/meta.yaml:37:    - pip check
docs/source/dev_guide/new_glb_plot.rst:13:- Run the `integration tests <https://e3sm-project.github.io/zppy/_build/html/main/dev_guide/testing.html#integration-tests>`_ and examine the differences from the expected files. If they match what you expect, update the expected files following "Commands to run to replace outdated expected files" on the machine-specific directions. (The commands to run all the integration tests are ``pip install .`` followed by ``python -u -m unittest tests/integration/test_*.py``).
docs/source/dev_guide/testing.rst:12:        pip install . # Install your changes (`python -m pip install .` also works)
docs/source/dev_guide/testing.rst:25:        pip install . # Install your changes (`python -m pip install .` also works)
docs/source/dev_guide/testing_e3sm_unified.rst:130:This will require running ``pip install .`` in that environment from the top level of
docs/source/getting_started.rst:162:Instead, the developer will ``python -m pip install .`` to build ``zppy`` with changes
docs/source/getting_started.rst:248:        pip install .

@xylar From #428:

You are right that the yaml file should also be updated to switch sphinx-multiversion to come from conda-forge, not pip. That seems like a trivial change.

I added a change to use mamba to install sphinx-multiversion but I'm not sure I did so correctly.

@xylar
Copy link
Contributor

xylar commented May 25, 2023

@forsyth2, my comment was referring to this yaml file:
https://github.com/E3SM-Project/zppy/blob/main/conda/dev.yml#L33-L34

Here, you will want to install all the packages currently being installed with pip using mamba instead, not just sphinx-multiversion. You want to avoid mixing conda-forge and pip packages whenever possible.

@@ -118,7 +118,8 @@ jobs:
- name: Install Dependencies
run: |
python -m pip install --upgrade pip
Copy link
Contributor

@xylar xylar May 25, 2023

Choose a reason for hiding this comment

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

Suggested change
python -m pip install --upgrade pip
mamba install -y sphinx==5.2.3 sphinx_rtd_theme==1.0.0 sphinx-multiversion==0.2.4 docutils==0.16

Comment on lines 121 to 122
pip install sphinx==5.2.3 sphinx_rtd_theme==1.0.0 docutils==0.16
mamba install sphinx-multiversion
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
pip install sphinx==5.2.3 sphinx_rtd_theme==1.0.0 docutils==0.16
mamba install sphinx-multiversion

@@ -38,7 +38,8 @@ jobs:
- name: Install Dependencies
run: |
python -m pip install --upgrade pip
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
python -m pip install --upgrade pip

Comment on lines 41 to 42
pip install sphinx==5.2.3 sphinx_rtd_theme==1.0.0 docutils==0.16
mamba install sphinx-multiversion
Copy link
Contributor

@xylar xylar May 25, 2023

Choose a reason for hiding this comment

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

Suggested change
pip install sphinx==5.2.3 sphinx_rtd_theme==1.0.0 docutils==0.16
mamba install sphinx-multiversion
mamba install -y sphinx==5.2.3 sphinx_rtd_theme==1.0.0 sphinx-multiversion==0.2.4 docutils==0.16

@xylar
Copy link
Contributor

xylar commented May 25, 2023

@forsyth2, here is how I am using mambaforge instead of miniconda in my GitHub Actions:
https://github.com/E3SM-Project/polaris/blob/main/.github/workflows/build_workflow.yml#L82

I'd suggest doing that here, too.

@forsyth2 forsyth2 requested review from xylar and tomvothecoder June 13, 2023 23:15
@forsyth2
Copy link
Collaborator Author

@tomvothecoder @xylar I think this pull request should be good to go. Please let me know if I'm missing anything. Thanks!

Copy link
Contributor

@xylar xylar left a comment

Choose a reason for hiding this comment

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

@forsyth2, sorry for the delay. I didn't realize this was waiting on me. Looks good to me.

Copy link
Collaborator

@tomvothecoder tomvothecoder left a comment

Choose a reason for hiding this comment

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

LGTM

@forsyth2 forsyth2 merged commit 3bce103 into main Jun 14, 2023
@forsyth2 forsyth2 deleted the issue-397 branch June 14, 2023 23:32
xylar pushed a commit to xylar/zppy that referenced this pull request Jun 16, 2023
forsyth2 added a commit that referenced this pull request Jun 16, 2023
* Change workflow to mamba (#429)

* Change workflow to mamba

* Fix GitHub Actions (#434)

* Fix GitHub Actions

* Revert sphinx-multiversion handling

* Get sphinx-multiversion from conda-forge

* Remove redundant dependency installation

* Switch docs workflows to mamba

---------

Co-authored-by: forsyth2 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DevOps CI/CD, configuration, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change workflow from pip to mamba/boa
3 participants