Support click v8.4#1260
Conversation
sirosen
requested review from
ada-globus,
derek-globus and
kurtmckee
as code owners
sirosen
requested review from
MaxTueckeGlobus,
aaschaer and
m1yag1
as code owners
8.4 introduces relatively few changes, but does make `ParamType` a Generic parametrized over the return type of `convert()`. Unfortunately, we also support Python 3.9 , which means we must support older click versions. As such, `ParamType` is not even subscriptable on that version. To work around this, we use `TYPE_CHECKING` guards to split definitions between a generic and non-generic variant. Only one line of runtime code is changed: A superfluous call to `super().convert()` was flagged by `mypy` for having the wrong type and is here removed.
sirosen
force-pushed
the
support-click-8.4
branch
from
b8358f5 to
c236c88
Compare
kurtmckee
left a comment
kurtmckee
left a comment
There was a problem hiding this comment.
It looks like click is continuing to release breaking changes in minor releases. We're on a treadmill.
I recommend that instead of "supporting click v8.4" this PR change to "require click v8.4". This is an application, and I think we can treat its dependencies as such by pinning them.
We already do this for a number of dependencies; I don't see a reason why click should be an outlier that requires a range of supported versions.
|
We discussed offline, but to surface in the permanent record:
We weren't able to drop old |
Now that we have removed support for click 8.3 and lower, `ParamType` is always generic and can have its type parameter specified directly. In one very specific case -- omittable datetimes -- this is specially handled with an explanatory comment.
This module is no longer needed now that our minimum version of click is 8.4 . Remove it and update all usages (which were structured to make this removal simple).
Searching for other `click`-version-sensitive code, outside of what is handled by `_click_compat`, we find one usage site, updated here.
Part of the documented signature of `convert()` is that `None` will never be passed to it. In spite of this, several `convert()` definitions in the CLI had inherited this handling from very old versions of `click`. Update to match current style. (NB: callbacks can still receive a `None`, which is used for various parameter definitions.)
`__call__` takes `str | None` and returns `UUID | None`. `convert()` takes `str` and returns `UUID`. Switching to use `convert()` has a positive effect.
|
I've done a full pass here, in independently reviewable commits. One side-effect of removing older |
2 participants
8.4 introduces relatively few changes, but does make
ParamTypeaGeneric parametrized over the return type of
convert(). Unfortunately,we also support Python 3.9 , which means we must support older click
versions. As such,
ParamTypeis not even subscriptable on that version.To work around this, we use
TYPE_CHECKINGguards to split definitionsbetween a generic and non-generic variant.
Only one line of runtime code is changed:
A superfluous call to
super().convert()was flagged bymypyforhaving the wrong type and is here removed.