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

CI: Install dvc via uv in the "GMT Tests" workflow #3695

Merged
merged 11 commits into from
Dec 19, 2024
Merged

Conversation

seisman
Copy link
Member

@seisman seisman commented Dec 18, 2024

dvc is a complex package that has ~40 direct dependencies (https://github.com/iterative/dvc/blob/007041bfe953db08b832ddea2d3adeaeb74eec95/pyproject.toml#L33). The complicated dvc dependency tree sometimes causes failures or version conflicts e.g., #2468, #2338, #1544.

In #3490, we're trying to add the Python 3.13 support to PyGMT. That PR is stale because some dvc dependencies don't have 3.13 support yet (#3490 (comment)), and we may have to wait for a few more days or months.

Since dvc is a dev dependency, we don't have to wait for the conda-forge updates. This PR tries to install dvc in other ways. I've tried the following options:

  1. Use https://github.com/iterative/setup-dvc
  2. Use pip
  3. Use pip on Linux/macOS and choco/winget on Windows
  4. Use uv

It turns out that installing dvc on Linux/macOS only takes ~10 seconds, but on Windows, options 1-3 take 60 seconds, except for option 4, which takes ~10 seconds on all platforms.

The CI job runs (https://github.com/GenericMappingTools/pygmt/actions/runs/12389452771/job/34582514750?pr=3695) for commit 83bb2d7 show that, with changes in this PR, we can continue the Python 3.13 support in PR #3490.

@seisman seisman force-pushed the python-3.13-dvc branch 2 times, most recently from c398edc to aa06f58 Compare December 18, 2024 08:43
@seisman seisman changed the title WIP: Explore ways to install dvc CI: Install dvc via uv in the "GMT Tests" workflow Dec 18, 2024
@seisman seisman added maintenance Boring but important stuff for the core devs skip-changelog Skip adding Pull Request to changelog needs review This PR has higher priority and needs review. labels Dec 18, 2024
@seisman seisman added this to the 0.14.0 milestone Dec 18, 2024
@seisman seisman requested a review from weiji14 December 18, 2024 09:19
Comment on lines +159 to +170
- name: Install dvc
run: |
uv venv
source .venv/bin/activate
uv pip install dvc
uv pip list

# Pull baseline image data from dvc remote (DAGsHub)
- name: Pull baseline image data from dvc remote
run: dvc pull --no-run-cache --verbose && ls -lhR pygmt/tests/baseline/
run: |
source .venv/bin/activate
uv run dvc pull --no-run-cache --verbose && ls -lhR pygmt/tests/baseline/
Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe we can combine the "Install dvc" and "Pull baseline image data from dvc remote" steps into one step "Setup DVC and pull baseline image data from dvc remote", so we don't have to source .venv/bin/activate twice.

Copy link
Member

Choose a reason for hiding this comment

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

Let's just keep this as is, since I'm assuming this will be a temporary workaround until conda-forge's dvc supports Python 3.13. We might want to revert the changes here after a few months, so best to have two separate steps.

Copy link
Member

@weiji14 weiji14 left a comment

Choose a reason for hiding this comment

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

Wish that setup-dvc was faster on Windows, but ok to go with this here.

Comment on lines +159 to +170
- name: Install dvc
run: |
uv venv
source .venv/bin/activate
uv pip install dvc
uv pip list

# Pull baseline image data from dvc remote (DAGsHub)
- name: Pull baseline image data from dvc remote
run: dvc pull --no-run-cache --verbose && ls -lhR pygmt/tests/baseline/
run: |
source .venv/bin/activate
uv run dvc pull --no-run-cache --verbose && ls -lhR pygmt/tests/baseline/
Copy link
Member

Choose a reason for hiding this comment

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

Let's just keep this as is, since I'm assuming this will be a temporary workaround until conda-forge's dvc supports Python 3.13. We might want to revert the changes here after a few months, so best to have two separate steps.

@seisman seisman removed the needs review This PR has higher priority and needs review. label Dec 19, 2024
@seisman seisman merged commit 4060160 into main Dec 19, 2024
16 checks passed
@seisman seisman deleted the python-3.13-dvc branch December 19, 2024 00:38
@weiji14 weiji14 mentioned this pull request Dec 19, 2024
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Boring but important stuff for the core devs skip-changelog Skip adding Pull Request to changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants