-
Notifications
You must be signed in to change notification settings - Fork 7
Correct the dev dependency table PEP 735 #126
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
Conversation
f8604fe
to
e2e6ed4
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.
This generally looks very good! Per usual, I have some suggestions for you to consider.
Co-authored-by: Christian Schou Oxvig <[email protected]>
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.
I have added all requested changes with only minor changes.
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.
A few more things to consider.
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.
I think this is ready for merge!
As I was trying to properly use the
uv sync
I had trouble to include the optional dependencies. This PR properly changes the label todependency-groups
which is the correct way to include developer dependencies according to PEP735. For discussion on differences between dependency-groups and optional-dependencies see this issue.Turns out
uv
support for PEP735 was released faster than forpip
. It is merged into main in the pip repository but does not currently seem to be released. We could change this PR to do the following solution or just update all the CI use uv and ignore the quick loss ofpip install -e .[dev]
notation.