-
Notifications
You must be signed in to change notification settings - Fork 64
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 to uv #170
Migrate to uv #170
Conversation
4ebb246
to
d4f5714
Compare
This reverts commit 44bf6f3.
Locking dependencies with "uv lock"... Using CPython 3.13.1 × No solution found when resolving dependencies for split (python_full_version >= '3.12'): ╰─▶ Because sentry-sdk==1.45.1 depends on urllib3==1.26.20 and urllib3==2.3.0, we can conclude that sentry-sdk==1.45.1 cannot be used. And because temporalio-samples:sentry depends on sentry-sdk==1.45.1 and your project requires temporalio-samples:sentry, we can conclude that your project's requirements are unsatisfiable. warning: An error occurred when locking dependencies, so "uv.lock" was not created. Locking dependencies with "uv lock" again to remove constraints... Using CPython 3.13.1 Resolved 113 packages in 44ms Successfully migrated project from Poetry to uv!
pyarrow install is failing
3152c08
to
8c6bcab
Compare
This reverts commit e7b3794.
8ffa011
to
725f277
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR migrates the project from using Poetry to using uv for dependency management and workflow execution. The key changes include:
- Updating run commands in README files from “poetry run” to “uv run” or “uv sync”.
- Adjusting CI configuration to use uv and its tooling.
- Wrapping the result from chain.ainvoke in langchain activities in a dict.
Reviewed Changes
Copilot reviewed 41 out of 41 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
message_passing/*/README.md | Updated commands to use uv instead of poetry |
langchain/activities.py | Changed return statement to wrap async result in a dict |
langchain/README.md, others | Updated sample instructions to reflect the uv commands |
.github/workflows/ci.yml | Updated CI configuration to install and sync using uv |
cloud_export_to_parquet/README.md | Updated dependency sync command with a modified group flag |
Comments suppressed due to low confidence (2)
langchain/activities.py:29
- Verify that wrapping the result of chain.ainvoke in a dict constructor is necessary; if chain.ainvoke already returns a dictionary, this extra conversion might be redundant or potentially introduce unexpected behavior.
return dict(
cloud_export_to_parquet/README.md:7
- [nitpick] Ensure consistency in group naming conventions; confirm whether the identifier should use dashes or underscores across the project.
uv sync --group=cloud-export-to-parquet
@copilot see 7bfe1d7. I believe dashes and underscores can be used interchangeably in group names, although a relevant uv bug was recently fixed: astral-sh/uv#12585. I propose using dashes here: it makes it clear to the reader that what they're looking at are not package names. |
|
||
## Usage | ||
|
||
Prerequisites: | ||
|
||
* Python >= 3.9 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we leave the Python version constraints?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't want users thinking they have to install a special Python version. uv users should install python using
uv python install 3.9
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy to clarify somewhere that they should run that command.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't want users thinking they have to install a special Python version. uv users should install python using
Right, "Python >= 3.9" just means "the software in this repository needs at least Python 3.9", it doesn't say how to install it or that it has to be any specific version. Our SDK and samples repo READMEs need to clarify which versions of Python they work with.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about this? c99cefe
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
.github/workflows/ci.yml
Outdated
- run: poetry install --with pydantic_converter --with dsl --with encryption --with trio_async | ||
- run: uv tool install poethepoet | ||
- run: uv sync --group=dsl --group=encryption --group=trio-async --python ${{ matrix.python }} | ||
- name: Check python version |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure we really need this Python version checker code (or the --python
on the uv sync
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops yes, removed.
Switch away from poetry to uv.
In the PR's current form, contributors will have to explicitly add samples to this list, at least if they are adding tests or something else that needs to import the sample package. I haven't yet found a way to automatically add top-level directories matching a mattern as packages, but nor have I confirmed that it is impossible. (I believe it's a difference between poetry and uv; the former did this automatically, and the latter perhaps views that as unprincipled.)