-
Notifications
You must be signed in to change notification settings - Fork 146
feat!: improve pyproject deps metatable #1464
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
base: master
Are you sure you want to change the base?
Conversation
typing-extensions is explicitly required in root. So it isn't needed in the dependency group
| "pytest-asyncio~=0.24.0", | ||
| "looptime~=0.2.0", | ||
| "coverage[toml]~=7.10.0", | ||
| "typing-extensions>=4.6", |
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 is in both groups because our tests use features from typing-extensions 4.6 or greater, while the main library only requires typing-extensions 4.1
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.
Should you not just bump up the minimum version of typing-extensions, then? I need to go through the changelog a bit more. But it might even be valid to go all the way up to 4.14.0. It would drop support for 3.8. And opens up the ability to consume 3.14 syntax.
That minimum version spec testing would be helpful here for validation... but either way, I see no reason to not raise the minimum. If you expect it to be required anyway for the types of things you do in the tests, it seems reasonable to expect that the end user will also need a compatible version (I should check the tests code though ...)
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.
That's fair. If we implement typing extensions get type hints (which we probably should) this becomes moot as we'll need to bump the minimum version a bit.
| @@ -0,0 +1 @@ | |||
| Explicitly use ``aiohttp[speedups]`` for ``speed`` extra. Simplifying the dependency chain. | |||
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.
Iirc the reason we didn't do this previously is SOMETHING breaks on this. It might be a bug in a different consumer but I need to check poetry. PDM, and so forth that they can correctly manage disnake as a dependency when this is done.
Summary
Closes #1461
Closes #1454
Warning
This PR is technically a breaking change
The docs extra should never be consumed by end users. However, technically, this would break anyone who did do that. Don't know why they would. But it could be the case.
aiohttp[speedups])Checklist
uv run nox -s lintuv run nox -s pyright