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

feat: replace poetry by uv #1

Merged
merged 9 commits into from
Jan 20, 2025
Merged

feat: replace poetry by uv #1

merged 9 commits into from
Jan 20, 2025

Conversation

gatheluck
Copy link

@gatheluck gatheluck commented Jan 17, 2025

Issue URL

cvpaperchallenge#106

Change overview

  • Replace poetry by uv (replace poetry.lock by uv.lock etc).
  • Update README.md
  • Update CI job for uv.
  • Update Dockerfile. Add some new env variables:
    • UV_TOOL_BIN_DIRECTORY: Define the directory path where uv Tools are installed. Used to specify UV_TOOL_BIN_DIR refered by uv.
    • INSTALL_UV_TOOLS: If true, install ruff, mypy, pytest etc as uv Tools.
    • RUN_UV_SYNC_AT_BUILD_TIME: Same as previous RUN_POETRY_INSTALL_AT_BUILD_TIME.

How to test

Please follow the instruction written in README.md. Note that poetry install command is now replaced by uv sync.

Note for reviewers

Note

In the uv, there is a concept called "Tools". Tools is a feature for managing tools used in development. For example, with Poetry, Ruff was installed as a dev dependency, and you would call it with commands like poetry run ruff <command>. However, by using the Tools feature in uv, you can invoke it directly with ruff <command>, eliminating the need to manage Ruff in pyproject.toml. Instead, tools are installed via uv tool install in the Dockerfile.

Note

We still using poethepoet as task runner. Task runner feature will be added as uv task in the future.
For detail, please check this issue.

@gatheluck gatheluck self-assigned this Jan 17, 2025
@gatheluck gatheluck changed the title feat: replace poetry by uv [WIP] feat: replace poetry by uv Jan 18, 2025
@gatheluck gatheluck marked this pull request as ready for review January 18, 2025 06:43
@gatheluck gatheluck changed the title [WIP] feat: replace poetry by uv feat: replace poetry by uv Jan 18, 2025
@gatheluck gatheluck added the enhancement New feature or request label Jan 18, 2025
Copy link

@Hina39 Hina39 left a comment

Choose a reason for hiding this comment

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

LGTM 🐨

Copy link

@YoshikiKubotani YoshikiKubotani left a comment

Choose a reason for hiding this comment

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

LGTM 💯

I will work on the revision of ci workflow (especially for caching) after this merge

@gatheluck gatheluck merged commit cc24101 into develop Jan 20, 2025
@chestnutforestlabo
Copy link

I tested it with my workstation, and it worked!

The code LGTM!!

While the below points may be irrelevant (and might be better to discuss in the other thread), I have heard some confusion from my friends, as explained below.

  1. Way to use uv
    Not all students can be familiar with uv (or even poetry).
    Maybe adding example use case like
uv run XXX.py

somewhere near the Start Development section may be user-friendly.

  1. Permission error:
virtualenv: error: argument dest: the destination . is not write-able at /home/challenger/ascender

I believe this error is very common for every user. (I faced it again this time lol)
I felt adding instructions to modify $UID and $GID near the Start Development can make the repo more nice.

Is there anything I can help with?

@gatheluck gatheluck mentioned this pull request Jan 22, 2025
@gatheluck
Copy link
Author

@chestnutforestlabo
Thank you for taking your time to check and raised suggestions! I created a new issue for them.

#2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants