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

migrate from poetry to uv #43

Merged
merged 24 commits into from
Nov 13, 2024
Merged

migrate from poetry to uv #43

merged 24 commits into from
Nov 13, 2024

Conversation

rbavery
Copy link
Collaborator

@rbavery rbavery commented Oct 31, 2024

Description

This migrate the toml and Makefile to use uv instead of poetry. In the Makefile, commands are all using --python $(which python) to enable installing dependencies in the active environment. To install a contained mlm project environment that integrates with the uv lockfile, don't use the Makefile and instead use uv sync and other uv commands without the --python flag.

I also amended the precommit config to allow precommit to install and run with any python version not just python 3.10. Otherwise, my test of making a commit while running precommit in a conda environment running 3.11 was breaking. All make commands have been tested successfull as using a fresh conda environment.

For some reason, the make pre-commit-install command is the only command that still sets up a .venv in the mlm repo even though --python $(which python) is used. However the conda python env is still used to run precommit.

→ make pre-commit-install   
uv is already installed
uv run --python /Users/ryanavery/micromamba/envs/test-gti/bin/python pre-commit install
Using CPython 3.11.10 interpreter at: /Users/ryanavery/micromamba/envs/test-gti/bin/python
Creating virtual environment at: .venv
Installed 78 packages in 129ms
pre-commit installed at .git/hooks/pre-commit

ruff catches over 80 issues and fixes most of them. I added these in this PR to check actions since I updated actions.

Type of Change

  • 📚 Examples, docs, tutorials or dependencies update;
  • 🔧 Bug fix (non-breaking change which fixes an issue);
  • 🥂 Improvement (non-breaking change which improves an existing feature);
  • 🚀 New feature (non-breaking change which adds functionality);
  • 💥 Breaking change (fix or feature that would cause existing functionality to change);
  • 🔐 Security fix.

Checklist

  • I've read the CONTRIBUTING.md guide;
  • I've updated the code style using make check;
  • I've written tests for all new methods and classes that I created;
  • I've written the docstring in Google format for all the methods and classes that I used.

Copy link
Collaborator

@fmigneault fmigneault left a comment

Choose a reason for hiding this comment

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

Looks like the linter checking markdown uses a default 80 char or something. There is a lot of unnecessary noise with newlines injected or moved to "fit" the contents when not needed.

I would prefer all changes introduced by this behavior to be reverted and have the tool "not care" about line length unless it goes beyond 120. The introduced noise causes reviews to be less focused on the important changes.

.gitignore Outdated Show resolved Hide resolved
.pre-commit-config.yaml Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README_STAC_MODEL.md Outdated Show resolved Hide resolved
README_STAC_MODEL.md Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@rbavery rbavery changed the title migrate from poetry to uv [wip] migrate from poetry to uv Nov 1, 2024
@rbavery
Copy link
Collaborator Author

rbavery commented Nov 1, 2024

The npm package versions in CI are

[email protected] /home/runner/work/mlm/mlm
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
└── [email protected]

which differs from package.json in the root of the repo

  "dependencies": {
    "remark-cli": "^8.0.0",
    "remark-lint": "^7.0.0",
    "remark-lint-no-html": "^2.0.0",
    "remark-gfm": "^4.0.0",
    "remark-preset-lint-consistent": "^3.0.0",
    "remark-preset-lint-markdown-style-guide": "^3.0.0",
    "remark-preset-lint-recommended": "^4.0.0",
    "remark-validate-links": "^10.0.0",
    "stac-node-validator": "^1.0.0"
  }

checks pass but running make format-markdown will still make stricter changes to markdown than we would like, for example adding extra spaces.

-> 1. Release for MLM specification (usually, this should include one for `stac-model` as well to support it)
-> 2. Release for `stac-model` only
+>
+> 1.  Release for MLM specification (usually, this should include one for `stac-model` as well to support it)
+> 2.  Release for `stac-model` only

I'm still not really sure why linting rules don't match formatting rules, I tried updating my local npm package versions to match ci, reinstall and rerun formatting and still got the same result. For now I suggest we don't use format-markdown to not block this PR because I think this issue is orthogonal to these changes.

@rbavery rbavery requested a review from fmigneault November 1, 2024 15:19
@rbavery rbavery changed the title [wip] migrate from poetry to uv migrate from poetry to uv Nov 1, 2024
.github/dependabot.yml Outdated Show resolved Hide resolved
.github/workflows/test.yaml Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
stac_model/input.py Outdated Show resolved Hide resolved
@rbavery rbavery requested a review from fmigneault November 5, 2024 18:46
@fmigneault fmigneault mentioned this pull request Nov 6, 2024
10 tasks
.github/dependabot.yml Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
Makefile Show resolved Hide resolved
@rbavery rbavery requested a review from fmigneault November 11, 2024 19:43
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
stac_model/input.py Outdated Show resolved Hide resolved
stac_model/input.py Outdated Show resolved Hide resolved
stac_model/input.py Outdated Show resolved Hide resolved
CONTRIBUTING.md Show resolved Hide resolved
stac_model/input.py Outdated Show resolved Hide resolved
stac_model/input.py Outdated Show resolved Hide resolved
@rbavery rbavery requested a review from fmigneault November 12, 2024 21:47
@fmigneault fmigneault merged commit 133b852 into main Nov 13, 2024
8 checks passed
@fmigneault fmigneault deleted the migrate-to-uv branch November 13, 2024 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants