openlawlibrary / pygls

A pythonic generic language server
https://pygls.readthedocs.io/en/latest/
Apache License 2.0
563 stars 102 forks source link

Consider removing upper bounds on dependencies #406

Open zanieb opened 10 months ago

zanieb commented 10 months ago

Since this is a library, including upper bounds on dependencies can be overly restrictive. Consumers of your library cannot use new major releases of dependencies, even if there are not breaking changes that affect this project. https://github.com/openlawlibrary/pygls/pull/405 is a good example. Removing upper bounds allows downstream applications using this library to upgrade without a new release of this library.

If you are interested in some content about this, the following blog posts by Henry Schreiner are quite comprehensive:

And here are posts from some members of the Python core developer team:

I'm happy to contribute this change if you are interested. The diff is quite minimal due to the limited dependencies

diff --git a/pyproject.toml b/pyproject.toml
index f061c37..2905d0f 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -15,8 +15,8 @@ readme = "README.md"
 [tool.poetry.dependencies]
 python = ">=3.7.9,<4"
 lsprotocol = "2023.0.0b1"
-typeguard = "^3.0.0"
-websockets = {version = "^11.0.3", optional = true}
+typeguard = ">=3.0.0"
+websockets = {version = ">=11.0.3", optional = true}

 [tool.poetry.extras]
 ws = ["websockets"]

I think keeping upper bounds on your development dependencies is reasonable — ideally those would be pinned anyway.

tombh commented 10 months ago

I actually looked into something very related to this when we were deciding whether to pin lsprotocol: https://github.com/openlawlibrary/pygls/issues/331 I'd be really interested to get your perspective on what I wrote.

But in short, I'm totally up for including that diff you posted.

dimbleby commented 10 months ago

IMO too much internet ink has been spilled on the topic of upper bounds. The trade-offs are pretty clear, but for an actively maintained project the downsides aren't so bad either way:

Either way there's an implicit burden on maintainers: either regularly to lift upper bounds, or to deal with the fall-out when breaking changes are made by dependencies.

Neither of these things happens very often, and for projects that are reasonably active none of their users will be in either of the bad places for very long anyway.

zanieb commented 10 months ago

Thanks for your replies :) I appreciate that this project is active.

IMO too much internet ink has been spilled on the topic of upper bounds.

I agree with that too, but because of Poetry's defaults many libraries include upper bounds resulting in unsolvable dependency trees without consideration.

While I agree there is some fallout when users encounter failures with incompatible dependencies, resolving the users issue is always the same: just install a compatible version. The alternative is that users have no recourse due to the limitations of Python tooling — as you said, they are hard blocked. In projects I've maintained, publishing a release is generally more work than responding to an issue.

There's also effect on the package ecosystem. When you include upper bounds, each published artifact is restricted forever. Although a new release can be made that increases the upper bound, the number of compatible releases is smaller which, in combination with other projects including bounds, can make package resolution quite difficult.

I hope to resolve these kinds of problems at its core, e.g. by improving the Python tooling itself, but until then I cannot justify the inclusion of upper bounds in libraries I publish.

An alternative is to stay on top of upgrading dependencies to reduce the number of overly restricted releases. You could use tooling to increase upper bounds as versions are released (typeguard-4.x was released quite some time ago). I'm not sure what dependabot support for poetry upper bounds is these days.

... deciding whether to pin lsprotocol: https://github.com/openlawlibrary/pygls/issues/331 I'd be really interested to get your perspective on what I wrote.

I agree with most of what you have there! Especially that pinning lsprotocol specifically makes sense for this library due to its specificity.

dimbleby commented 10 months ago

because of Poetry's defaults many libraries include upper bounds resulting in unsolvable dependency trees without consideration.

In my experience this is a great exaggeration of the number of unsolvable dependency trees out there; but perhaps your experience is different.

However it would not be very consistent of me to note that too much has already been said on this topic, and opine that it does not much matter anyway - and then get into a to-and-fro about it! So I will leave it there.

zanieb commented 10 months ago

I wish it was not a problem I encountered so often :) I appreciate hearing your experience and opinion though!

I don't think much more needs to be said for this library in particular. Whatever the maintainers are most comfortable with seems reasonable, as this package has few dependencies and is probably best installed in isolated environments regardless.

tombh commented 10 months ago

Thanks for the input, it's all great information to help build up a broader and more detailed picture.

And of course happy to continue to hear more views on this.

tombh commented 9 months ago

I think that's reasonable. I'm happy to accept any PRs around this.