python / typeshed

Collection of library stubs for Python, with static types
Other
4.15k stars 1.69k forks source link

Set up automation for updating pytype and pyright versions #11484

Closed JelleZijlstra closed 3 months ago

JelleZijlstra commented 4 months ago

Both pytype and pyright release pretty frequently and it's useful for us to make sure we test against the latest version. It would be nice to have a GitHub action that automatically updates them for us so we don't have to remember to make a PR. Example PRs: #11483, #11190.

I suppose other dependencies could also be auto-updated but the need isn't as great.

AlexWaygood commented 4 months ago

We could probably do this fairly easily with dependabot. We wouldn't need to do it with all our dependencies if we don't want to; but also, dependabot now supports setting dependency groups that are all updated together, so it wouldn't be especially noisy to do it for all our dependencies

JelleZijlstra commented 4 months ago

I think I'd probably want new versions of mypy, pyright, and pytype to trigger an immediate (daily) update PR, and everything else can get updated every month.

Avasam commented 4 months ago

I don't know if dependabot can detect our deduplicated pyright version though. If it's automatically handled, it would be fine to have it duplicated again (and maybe use the python wrapper as a dependency in requirements-tests.txt, but even then I'm not sure if the version given as an argument to pyright-action will be picked up.

Edit: Relevant conversation: https://github.com/jakebailey/pyright-action/issues/9#issuecomment-1629781059

Mr-Sunglasses commented 4 months ago

Hey @JelleZijlstra, I created a script that updates dependencies daily and creates a PR. Let me know if this matches our goal and if we need more improvements.

JelleZijlstra commented 4 months ago

Sounds great, thank you! We'll review your PR soon.

jakebailey commented 4 months ago

FWIW a strategy that can be used for pyright is to have a package.json in the repo with an exact version, then pull the value out (kinda like you do with the toml now). That's the stragey that we use on TypeChat's python package: https://github.com/microsoft/TypeChat/blob/main/python/package.json#L12

You don't need to actually install it via that mechanism if you don't want.

(I am tempted to just have the pyright-action read out of a package.json, per the above linked request)

AlexWaygood commented 3 months ago

We now have four potential solutions prototyped, and I think it's time to decide between them:

  1. Carry on pinning dependencies the way we do now, and use a custom script to auto-update our dependencies: https://github.com/python/typeshed/pull/11491
  2. Move our pyright pin to a package.json file and use dependabot to auto-update our dependencies: https://github.com/python/typeshed/pull/11564
  3. Move our pyright pin to requirements.txt and use dependabot to auto-update our dependencies: https://github.com/python/typeshed/pull/11575
  4. Carry on pinning dependencies the way we do now, and use renovate to auto-update our dependencies: https://github.com/python/typeshed/pull/11565

I think all four proposals have their pros and cons. Of the four, I like (4) best, then (3). I like (4) because:

I have some issues with adding pyright to our requirements.txt file (briefly outlined in https://github.com/python/typeshed/pull/11575#issuecomment-1988884414 / https://github.com/python/typeshed/pull/11575#issuecomment-1988985874) but I think https://github.com/python/typeshed/pull/11575 is a clear improvement over https://github.com/python/typeshed/pull/11564; it's definitely preferable to put our pyright dependency pin in one of our existing config files rather than a "fake" package.json file, in my opinion.

I like https://github.com/python/typeshed/pull/11491 the least out of our currently prototyped options, as I'd prefer to go with a widely used, well-tested tool rather than a custom script.

srittau commented 3 months ago

Personally, I like (3) best, followed by (4), if(!) the issues mentioned in #11575 can be ironed out in a non-flaky way. Adding pyright to requirements.txt has the advantage that all type checker versions are together in one file, and that users get a full test environment, just by running pip install -r requirements.txt.

Avasam commented 3 months ago

Whilst we're on the discussion, do we want to consider handling dependencies updates shared by requirements-test.txt and .pre-commit-config.yaml together? Sure there's a comment (and a think a custom check?) that makes it so if it's automatically updated in one file, we should remember to update it in the other file as well. But it's be nice if our final solution allows both to be handled together automatically, no manual change needed (except when the workflow fails and manual changes to code is needed of course)

AlexWaygood commented 3 months ago

Whilst we're on the discussion, do we want to consider handling dependencies updates shared by requirements-test.txt and .pre-commit-config.yaml together?

Renovate can update pre-commit config files, though there's a note in the renovate docs saying that the pre-commit maintainers don't like renovate and prefer you to use the pre-commit auto-update bots. I can update https://github.com/python/typeshed/pull/11565 so renovate updates our pre-commit config as well, if that's something we want. I agree it would be nice to have them updated together, and I can't remember the exact details of the pre-commit/renovate argument

JelleZijlstra commented 3 months ago

There's a lot of things the pre-commit maintainers don't like, though it's probably worth figuring out the exact reason for the recommendation here. If Renovate works, I don't see a reason not to use it for pre-commit too.

JelleZijlstra commented 3 months ago

Seems like this is the context: https://github.com/renovatebot/renovate/issues/11166#issuecomment-896236031. Some disagreement about whether to autoupdate the minimum required pre-commit version.

srittau commented 3 months ago

Of course being able to update pre-commit as well is a big plus for renovate. In that regard, #11565 and #11575 are not mutually exclusive. The latter would work both with dependabot and renovate, but is a requisite for dependabot.

Avasam commented 3 months ago

The fact that we can somewhat easily achieve linked dependencies upgrade across files with Renovate, and that it seems a bit more future-proof and featureful than Dependabot, makes me lean in its favor.

For having used Renovate myself, I especially like all the stats and references that come with PR descriptions.

srittau commented 3 months ago

So it seems the best option is to go with renovate (#11565) and optionally do #11575 if we get it to work.

AlexWaygood commented 3 months ago

In that regard, #11565 and #11575 are not mutually exclusive.

That hadn't occured to me, but you're quite right!

So it seems the best option is to go with renovate (#11565) and optionally do #11575 if we get it to work.

That makes sense to me. I'll try to update that PR today so that it updates pre-commit dependencies as well.

AlexWaygood commented 3 months ago

We're now successfully setup with renovate 🎉:

The only thing it looks like it won't do automatically is update our flake8-pyi pin in .pre-commit-config.yaml (because it's listed as an "additional dependency" of the flake8 hook rather than a hook in and of itself)