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 to use uv instead of poetry #2288

Merged
merged 3 commits into from
Jan 30, 2025
Merged

Conversation

gaborbernat
Copy link
Collaborator

@gaborbernat gaborbernat commented Jan 29, 2025

Use hatchling instead of poetry for backend, uv otherwise.

Resolves #2220

Signed-off-by: Bernát Gábor <bgabor8@bloomberg.net>

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Signed-off-by: Bernát Gábor <bgabor8@bloomberg.net>
Signed-off-by: Bernát Gábor <bgabor8@bloomberg.net>
@gaborbernat gaborbernat changed the title main Migrate to use uv instead of poetry Jan 29, 2025
@gaborbernat
Copy link
Collaborator Author

@koxudaxi any update on this? I have another bug I'd like to addrtess, and this is blocking me 😊

Copy link

codecov bot commented Jan 30, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (dd92cd0) to head (125b76b).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##              main     #2288    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files           38        38            
  Lines         4307      4305     -2     
  Branches       994       838   -156     
==========================================
- Hits          4307      4305     -2     
Flag Coverage Δ
unittests 99.67% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

codspeed-hq bot commented Jan 30, 2025

CodSpeed Performance Report

Merging #2288 will improve performances by 11.57%

Comparing gaborbernat:main (125b76b) with main (dd92cd0)

🎉 Hooray! pytest-codspeed just leveled up to 3.1.2!

A heads-up, this is a breaking change and it might affect your current performance baseline a bit. But here's the exciting part - it's packed with new, cool features and promises improved result stability 🥳!
Curious about what's new? Visit our releases page to delve into all the awesome details about this new version.

Summary

⚡ 3 improvements
✅ 28 untouched benchmarks

Benchmarks breakdown

Benchmark BASE HEAD Change
test_main_jsonschema_collapsed_external_references 80.9 ms 72.5 ms +11.57%
test_main_jsonschema_multiple_files 82 ms 74.4 ms +10.24%
test_main_jsonschema_no_empty_collapsed_external_model 51 ms 46.2 ms +10.44%

@gaborbernat
Copy link
Collaborator Author

gaborbernat commented Jan 30, 2025

Ah, I slighthly renamed the GitHub actions so shows more info in my browser without needing to click into it, which causes the missing checks. Should I restore the old naming or you OK updating the required checks?

@koxudaxi
Copy link
Owner

Thank you very much for creating PR!!
Please keep the current naming as is, and I will update the required check names after merging.
It seems the coverage has decreased, so could you please fix only that part?

@gaborbernat
Copy link
Collaborator Author

Hmm, unsure why the coverage decreased, I haven't touched any of those files 🤔

@gaborbernat
Copy link
Collaborator Author

Orthogonal to this change set, but given that uv does not yet support a task manager, would you be open to add tox-uv to this project, so easier to setup these envs locally? 😊 this would replace the shell scripts also. uv plans to add task manager support later this year at which point we could swap over.

@koxudaxi
Copy link
Owner

Looking at https://app.codecov.io/gh/koxudaxi/datamodel-code-generator/pull/2288/indirect-changes#6e0ac031c1cc8fb390d8111b0e1d795b-L194,
it seems the isort4.x version is not covered.
However, checking the logs at https://github.com/koxudaxi/datamodel-code-generator/actions/runs/13039215945/job/36434145108?pr=2288#step:5:15,
it appears isort4.x is installed correctly 🤔

I apologize, but it's getting late here so I will check this again tomorrow.

Regarding tox-uv, while it's welcome, I will also read the documentation tomorrow myself.

@gaborbernat
Copy link
Collaborator Author

Regarding tox-uv, while it's welcome, I will also read the documentation tomorrow myself.

Disclaimer I am the auther and maintainer of both tox and tox-uv; so if you have any questions you can ask 😊 Yeah I saw the same, so will need to check. Will put in a commit to add tox-uv (as helps me to try to reproduce this easier locally) and circle back.

@koxudaxi koxudaxi merged commit 107168c into koxudaxi:main Jan 30, 2025
28 checks passed
@koxudaxi
Copy link
Owner

The coverage was fixed after re-running only that specific task.
It's a very silly phenomenon😣
Thank you. I'll be back tomorrow to handle this and other PRs.
I'll leave another message here or somewhere else.
Thank you very much for your cooperation!!

@gaborbernat
Copy link
Collaborator Author

Another orthogonal question, would you be open to switch to a src layout? 😆 https://packaging.python.org/en/latest/discussions/src-layout-vs-flat-layout/

@skshetry
Copy link

skshetry commented Feb 7, 2025

This PR seems to have changed a lot of dependencies to be required that were optional before:

prance = { version = ">=0.18.2", optional = true }
openapi-spec-validator = { version = ">=0.2.8,<0.7.0", optional = true }
PySnooper = { version = ">=0.4.1,<2.0.0", optional = true }
httpx = { version = "*", optional = true }
graphql-core = {version = "^3.2.3", optional = true}

Is this intentional or an oversight? My Apologies, if this has been discussed before, but I could not find it.

@gaborbernat
Copy link
Collaborator Author

Not sure what optional dependency is for Poetry. Core Python doesn't have that concept only extras. How did poetry map these to standard dependencies?

@skshetry
Copy link

skshetry commented Feb 7, 2025

@gaborbernat, PTAL at Extras - poetry.

My understanding reading that is, in poetry, you use it to specify version constraints for optional dependency, which later you are going to use in extras. And looks like it's a deprecated way of specifying extra/optional dependencies.

I think you could simply get rid of these dependencies from project.dependencies as they are already set in project.optional-dependencies.

@koxudaxi
Copy link
Owner

koxudaxi commented Feb 7, 2025

@skshetry
Thank you for your report.
I'm working on the PR

#2306

@koxudaxi
Copy link
Owner

koxudaxi commented Feb 7, 2025

OK, I have released the fixed version 0.27.2

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.

Migrate from Poetry to uv package manager
3 participants