Update click to 8 #432

DavidOliver · 2021-05-20T13:20:39Z

Warning: I don't know what I'm doing; I don't usually use Python and this is my first time with Watson's code, so this pull request should be treated with caution. But hopefully it can at least act as the start of a fix for use of click v8.

The cause of #430 seems to be this (click v8 changes):

TypeError is raised when parameter with multiple=True or nargs > 1 has non-iterable default. #1749

'Multiple options' at click documentation.

I remove multiple=True from several click options, which I don't believe are intended to be specified at the command line more than once as they seem to be flags rather than options which take (multiple) values.

I believe I've successfully run Watson's tests, which all seem to be passing.

By the way, will a new release of Watson be prepared soon, either with the temporary fix of specifying v7 of click or this fix? I ask as I'm currently working on my system config. :)

jmaupetit · 2021-05-20T15:07:11Z

Wow this is huge. Thanks 🙏 We'll look into this soon.

k4nar · 2021-05-26T10:59:07Z

requirements.txt

@@ -1,5 +1,5 @@
 arrow>=1.0.0
-click>=7.0,<8.0
+click>=8.0,<9.0


I don't think we should explicitly forbid 9.0, we don't know if that will be a breaking change for us yet. We usually try to avoid pinning versions if we can.

I restricted to 8 partly because it was a "MAJOR" semver version change (7 to 8) that broke Watson. I'm not familiar with either project, but shouldn't major releases of dependencies only be (explicitly) updated to once they've been tested due to the likelihood of breaking changes?

This is a long debate in the Python community :) . So far my opinion is to allow any version of a dependency as long as you don't know for sure that you're not compatible with it. This is less of an issue if you isolate the dependencies of each Python project you're using, but if you're not doing it things can get messy pretty quickly if everybody is asking for specific versions of a package for no proper reason.

Huh - interesting! Seems like asking for trouble to me. :D Will you remove the <9 requirement or should I?

Hi David! Can you fix this, and then I'll merge your work and release it. Thanks!

Thanks! Done.

k4nar

Indeed I don't think those should have been marked as "multiple", this was probably a bad copy/paste in the first place 😄 .

This looks to me, except for the small comment about the pinned version 👍 . Thank you very much!

mizhozan · 2021-05-26T14:38:27Z

I'm not pro Python user and don't much about test suits, but my test with this pr on my local computer works fine plus I did a simple pytest and there was 250 passed, 14 warning, 91 errors as following gist.

I simply did a gh pr checkout 432 and then pip install and check watson which works just fine. For the test I just did pytest from the command line, if it's supposed (didn't see the documentation on test).

DavidOliver · 2021-05-26T15:04:13Z

@mizhozan, I also had those fixture-related failures. The solution seemed to be install the dev requirements from requirements-dev.txt. (See the varying mock versions depending on the version of setuptools you have.)

mizhozan · 2021-05-27T04:18:32Z

@DavidOliver Thanks a lot, now the test runs perfectly with 341 passes and 14 warnings according to this gist.

@mizhozan, I also had those fixture-related failures. The solution seemed to be install the dev requirements from requirements-dev.txt. (See the varying mock versions depending on the version of setuptools you have.)

As requested by maintainers.

DavidOliver · 2021-05-27T10:16:31Z

@mizhozan Thanks. I think I managed to confuse myself regarding the versions of packages I had installed. I've just (re-?)re-installed click (8.0.1), and now I also have the deprection warnings when running the tests.

@jmaupetit Can you hold off on merging, please? I'll see if I can get rid of the deprecation warnings.

DavidOliver · 2021-05-27T11:10:07Z

The deprecation warnings in the tests are now gone (see last commit), but I'm out of my depth here and can't know whether or not more needs to be done to properly update to click 8.

From the changelog:

Redesign the shell completion system. #1484, #1622

Support Bash >= 4.4, Zsh, and Fish, with the ability for extensions to add support for other shells.

Allow commands, groups, parameters, and types to override their completions suggestions.

Groups complete the names commands were registered with, which can differ from the name they were created with.

The autocompletion parameter for options and arguments is renamed to shell_complete. The function must take ctx, param, incomplete, must do matching rather than return all values, and must return a list of strings or a list of CompletionItem. The old name and behavior is deprecated and will be removed in 8.1.

The env var values used to start completion have changed order. The shell now comes first, such as {shell}source rather than source{shell}, and is always required.

Regarding shell_complete, it looks like the args taken may not need updating, but I don't know about "matching rather than return all values" and the return values.

Regarding env var values to start completion, I've searched for source_ in Watson's code and found source_zsh in create-completion-script.sh. I think this is where I bow out and leave things for others with more knowledge and understanding of Watson and Python. Thanks!

pythonPackages brings click 8. Unfortunately, watson is not ready to consume this library yet. Fix by explicitly pulling in click 7 as dependency. See these upstream discussions: TailorDev/Watson#430 TailorDev/Watson#432

MinchinWeb · 2021-09-02T23:40:50Z

Ran into this issue today. Would love to see this merged in.

Xiretza · 2022-05-15T11:04:56Z

It's been almost a year, what's the status on this?

jmaupetit · 2022-05-16T10:20:10Z

Rebased and merged in #471

Update click to 8

e959d2b

jmaupetit requested review from jmaupetit and k4nar May 20, 2021 15:05

jmaupetit added bug critical improvement and removed bug labels May 20, 2021

k4nar reviewed May 26, 2021

View reviewed changes

Remove pinning of click to version 8

abde84a

As requested by maintainers.

Use click’s new parameter name of ‘shell_complete’

038fe57

lepokle mentioned this pull request Jun 7, 2021

Cannot start watson anymore #434

Closed

blitz mentioned this pull request Jul 9, 2021

watson: fix build problem with click 8 NixOS/nixpkgs#129755

Merged

11 tasks

ab2422 mentioned this pull request Aug 9, 2021

Crash in virtual env on Windows 10, in powershell #448

Closed

davinellulinvega mentioned this pull request Sep 10, 2021

[BUG] watson crashes at start after some update #451

Closed

jmaupetit mentioned this pull request May 16, 2022

Fix click 8 compatibility issues #471

Merged

jmaupetit closed this May 16, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update click to 8 #432

Update click to 8 #432

DavidOliver commented May 20, 2021

jmaupetit commented May 20, 2021

k4nar May 26, 2021

DavidOliver May 26, 2021 •

edited

k4nar May 26, 2021

DavidOliver May 26, 2021

jmaupetit May 27, 2021

DavidOliver May 27, 2021

k4nar left a comment

mizhozan commented May 26, 2021 •

edited

DavidOliver commented May 26, 2021

mizhozan commented May 27, 2021 •

edited

DavidOliver commented May 27, 2021 •

edited

DavidOliver commented May 27, 2021 •

edited

MinchinWeb commented Sep 2, 2021

Xiretza commented May 15, 2022

jmaupetit commented May 16, 2022

Update click to 8 #432

Update click to 8 #432

Conversation

DavidOliver commented May 20, 2021

jmaupetit commented May 20, 2021

k4nar May 26, 2021

Choose a reason for hiding this comment

DavidOliver May 26, 2021 • edited

Choose a reason for hiding this comment

k4nar May 26, 2021

Choose a reason for hiding this comment

DavidOliver May 26, 2021

Choose a reason for hiding this comment

jmaupetit May 27, 2021

Choose a reason for hiding this comment

DavidOliver May 27, 2021

Choose a reason for hiding this comment

k4nar left a comment

Choose a reason for hiding this comment

mizhozan commented May 26, 2021 • edited

DavidOliver commented May 26, 2021

mizhozan commented May 27, 2021 • edited

DavidOliver commented May 27, 2021 • edited

DavidOliver commented May 27, 2021 • edited

MinchinWeb commented Sep 2, 2021

Xiretza commented May 15, 2022

jmaupetit commented May 16, 2022

DavidOliver May 26, 2021 •

edited

mizhozan commented May 26, 2021 •

edited

mizhozan commented May 27, 2021 •

edited

DavidOliver commented May 27, 2021 •

edited

DavidOliver commented May 27, 2021 •

edited