Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Replaced pdm with uv#10727

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
sydney-runkle merged 29 commits intopydantic:mainfromfrfahim:pdm-to-uv
Nov 8, 2024
Merged

Replaced pdm with uv#10727

sydney-runkle merged 29 commits intopydantic:mainfromfrfahim:pdm-to-uv
Nov 8, 2024

Conversation

frfahim
Copy link
Contributor

@frfahimfrfahim commentedOct 27, 2024•
edited
Loading

Change Summary

This pull request is replacing thepdm package manager withuv in the CI workflows and other related configurations. The changes ensure that the new package manager is correctly set up and used across various jobs and scripts.
Makefile: Replacedpdm commands withuv for tasks such as installing dependencies, running tests, and generating documentation.

Related issue number

Fix:#10637

Checklist

  • The pull request title is a good summary of the changes - it will be used in the changelog
  • Unit tests for the changes exist
  • Tests pass on CI
  • Documentation reflects the changes where applicable
  • My PR is ready to review,please add a comment including the phrase "please review" to assign reviewers

sydney-runkle, benedikt-bartscher, and yangshangbo reacted with heart emoji
@github-actionsgithub-actionsbot added the relnotes-fixUsed for bugfixes. labelOct 27, 2024
@codspeed-hqCodSpeed HQ
Copy link

codspeed-hqbot commentedOct 27, 2024•
edited
Loading

CodSpeed Performance Report

Merging#10727 willnot alter performance

Comparingfrfahim:pdm-to-uv (7a654d2) withmain (71c087c)

Summary

✅ 44 untouched benchmarks

@github-actionsGitHub Actions
Copy link
Contributor

github-actionsbot commentedOct 29, 2024•
edited
Loading

Coverage report

This PR does not seem to contain any modification to coverable code.

Copy link
Contributor

@sydney-runklesydney-runkle left a comment

Choose a reason for hiding this comment

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

Looking great, thanks so much for your help here!

I've got a few questions / change requests!

@pydantic-hookypydantic-hookybot added the awaiting author revisionawaiting changes from the PR author labelOct 31, 2024
@sydney-runkle
Copy link
Contributor

Very odd that we're seeing benchmark changes here...

@Viicos
Copy link
Member

Very odd that we're seeing benchmark changes here...

Benchmarks seem to be broken as they did not run and thus codspeed is for some reason reporting non sense results

sydney-runkle reacted with thumbs up emoji

@sydney-runkle
Copy link
Contributor

Maybe bumping to v3.0.0 will help...

@frfahim
Copy link
ContributorAuthor

frfahim commentedNov 2, 2024•
edited
Loading

Very odd that we're seeing benchmark changes here...

I don't have any clue why benchmarks are changing here, I was looking for a fix.

sydney-runkle reacted with heart emoji

Copy link
Contributor

@sydney-runklesydney-runkle left a comment

Choose a reason for hiding this comment

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

Fixing the codspeed issue, hopefully :)

frfahim reacted with rocket emoji
@frfahim
Copy link
ContributorAuthor

There is a conflict with pdm lock file, should I rebase it?
btw, CI's are not running.

sydney-runkle reacted with heart emoji

@sydney-runkle
Copy link
Contributor

All good, I can take it from here. Thanks!

frfahim reacted with thumbs up emoji

run: |
pdm venv create --with-pip --force $PYTHON
pdm install -G docs
run: uv sync --python 3.12 --group docs
Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

uv automatically checksrequires-python constraints and use that python version. Is it necessary to mention the python version again here?

Copy link
Contributor

Choose a reason for hiding this comment

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

It does seem redundant...

Copy link
Member

Choose a reason for hiding this comment

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

So the thing isuv will prefer the specified version in the.python-version file if present, no matter the previously installed version. We currently don't have such a file, but it could be pretty bad if we end up creating one at some point, especially for the jobs with a Python version matrix: CI will only run on the version from.python-version, and we won't notice anything.

It's a bit unfortunate, probably using tox (and tox-uv) could help.

sydney-runkle reacted with thumbs up emoji
Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

when a python version matrix exists, then python version is specified with the sync command
uv sync --python ${{ matrix.python-version }}

So it will use the requested python version from matrix even the.python-version file exist

sydney-runkleand others added3 commitsNovember 5, 2024 17:32
Co-authored-by: Farhadur Reza Fahim <farhadur.fahim@gmail.com>
Co-authored-by: Farhadur Reza Fahim <farhadur.fahim@gmail.com>
Co-authored-by: Farhadur Reza Fahim <farhadur.fahim@gmail.com>
@Viicos
Copy link
Member

Seems like in some CI jobs, we runuv install ... and thenuv sync ..., and in others we only runuv sync ....uv sync should calluv install right? Maybe it's best to be explicit and have auv install ... step for every job

@frfahim
Copy link
ContributorAuthor

Seems like in some CI jobs, we runuv install ... and thenuv sync ...

uv sync command is used everywhere, there is no use ofuv install command in the CI's

@sydney-runklesydney-runkle added relnotes-packagingUsed for dependency changes. and removed awaiting author revisionawaiting changes from the PR author relnotes-fixUsed for bugfixes. labelsNov 7, 2024
@sydney-runkle
Copy link
Contributor

I've removed the unnecessary installs :)

frfahim reacted with heart emoji

@sydney-runkle
Copy link
Contributor

Unclear to me why that test is failing... investigating now.

frfahim reacted with eyes emoji

@sydney-runkle
Copy link
Contributor

@frfahim,

Any chance you could take a look here?

@frfahim
Copy link
ContributorAuthor

Sure, investigating the issue 👀

sydney-runkle reacted with heart emoji

@sydney-runkle
Copy link
Contributor

Looks like we're installing an older version of Python 3.9 - not sure why - previously, it was3.9.20, now it's3.9.6 without the manualinstall before thesync.

@frfahim
Copy link
ContributorAuthor

frfahim commentedNov 7, 2024•
edited
Loading

It's using system python version, for macos 13 (with 3.9) it is 3.9.6

Run uv sync --python 3.9 --group testing --extra timezone
Using CPython 3.9.6 interpreter at: /Applications/Xcode_15.2.app/Contents/Developer/usr/bin/python3

It looks like it's not using uv installed python version for macos 13 with 3.9

This command will fix the issue, but I am looking for a better solution
uv sync --python ${{ matrix.python-version }} --python-preference only-managed

sydney-runkle reacted with heart emoji

Copy link
Contributor

@sydney-runklesydney-runkle left a comment

Choose a reason for hiding this comment

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

Thanks so much!! Great work!

frfahim reacted with heart emoji
@sydney-runklesydney-runkle merged commit53bf2f2 intopydantic:mainNov 8, 2024
54 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@ViicosViicosViicos left review comments

@sydney-runklesydney-runklesydney-runkle approved these changes

Assignees

@frfahimfrfahim

Labels
relnotes-packagingUsed for dependency changes.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Upgrade touv instead ofpdm
3 participants
@frfahim@sydney-runkle@Viicos
[8]ページ先頭
©2009-2025 Movatter.jp