Change in isort usage from 4 to 5 is a bit unexpected

[karrtikr]

Hi VSCode dev here 👋

isort 4.3.21

usage: isort [-h] [-a ADD_IMPORTS] [-ac] [-af] [-b KNOWN_STANDARD_LIBRARY] [-c] [-ca] [-cs] [-d] [-df] [-ds] [-dt] [-e] [-f KNOWN_FUTURE_LIBRARY] [-fas] [-fass]
             [-ff FROM_FIRST] [-fgw [FORCE_GRID_WRAP]] [-fss] [-i INDENT] [-j JOBS] [-k] [-l LINE_LENGTH] [-lai LINES_AFTER_IMPORTS] [-lbt LINES_BETWEEN_TYPES]     
             [-le LINE_ENDING] [-ls] [-m {0,1,2,3,4,5,6,7}] [-nis] [-nlb NO_LINES_BEFORE] [-ns NOT_SKIP] [-o KNOWN_THIRD_PARTY] [-ot] [-p KNOWN_FIRST_PARTY] [-q]   
             [-r] [-rm REMOVE_IMPORTS] [-rr] [-rc] [-s SKIP] [-sd DEFAULT_SECTION] [-sg SKIP_GLOB] [-sl] [-sp SETTINGS_PATH] [-t FORCE_TO_TOP] [-tc] [-up] [-v]     
             [-vb] [--virtual-env VIRTUAL_ENV] [--conda-env CONDA_ENV] [-vn] [-w LINE_LENGTH] [-wl WRAP_LENGTH] [-ws] [-y] [--unsafe] [--case-sensitive]
             [--filter-files]
             [files [files ...]]

isort 5.2.2

usage: isort [-h] [--src SRC_PATHS] [-a ADD_IMPORTS] [--append] [--ac] [--af] [-b KNOWN_STANDARD_LIBRARY] [--extra-builtin EXTRA_STANDARD_LIBRARY] [-c] [--ca]
             [--cs] [-d] [--df] [--ds] [-e] [-f KNOWN_FUTURE_LIBRARY] [--fas] [--fass] [--ff FROM_FIRST] [--fgw [FORCE_GRID_WRAP]] [--fss] [-i INDENT] [-j JOBS]    
             [--lai LINES_AFTER_IMPORTS] [--lbt LINES_BETWEEN_TYPES] [--le LINE_ENDING] [--ls]
             [-m {GRID,VERTICAL,HANGING_INDENT,VERTICAL_HANGING_INDENT,VERTICAL_GRID,VERTICAL_GRID_GROUPED,VERTICAL_GRID_GROUPED_NO_COMMA,NOQA,VERTICAL_HANGING_INDENT_BRACKET,VERTICAL_PREFIX_FROM_MODULE_IMPORT,HANGING_INDENT_WITH_PARENTHESES,0,1,2,3,4,5,6,7,8,9,10}]
             [-n] [--nis] [--nlb NO_LINES_BEFORE] [-o KNOWN_THIRD_PARTY] [--ot] [--dt] [-p KNOWN_FIRST_PARTY] [--known-local-folder KNOWN_LOCAL_FOLDER] [-q]        
             [--rm REMOVE_IMPORTS] [--rr] [-s SKIP] [--sd DEFAULT_SECTION] [--sg SKIP_GLOB] [--gitignore] [--sl] [--nsl SINGLE_LINE_EXCLUSIONS]
             [--sp SETTINGS_PATH] [-t FORCE_TO_TOP] [--tc] [--up] [-V] [-v] [--virtual-env VIRTUAL_ENV] [--conda-env CONDA_ENV] [--vn] [-l LINE_LENGTH]
             [--wl WRAP_LENGTH] [--ws] [--case-sensitive] [--filter-files] [--py {all,2,27,3,35,36,37,38,39,auto}] [--profile PROFILE] [--interactive]
             [--old-finders] [--show-config] [--honor-noqa] [--remove-redundant-aliases] [--color] [--float-to-top] [--formatter FORMATTER]
             [files [files ...]]

For instance, settings path is now specified using --sp instead of -sp. We have a setting python.sortImports.args through which users pass arguments to the isort command we run. If we upgrade isort it'll break settings for all those users as many old flags are no longer valid.

I was wondering what's the reason for such changes and if it's possible to roll them back.

[timothycrosley]

Hi @karrtikr!

Thanks for the work on VSCode! The reason is simply that single dash arguments with multiple letters can cause ambiguity:

Normally you can chain single dash arguments with one dash. So:

isort . -cv

Is the same as isort . -c -v or isort . --check --verbose. However, isorts previous inclusion of multi characters single dash arguments, breaks this expectation of CLI tools.

Let's say -r stands for remove all imports and -c stands for check, with -rc standing for --recursive.
It then becomes ambiguous to the user what will happen when isort -rc is called. It could either remove all imports and then check if that produces differences, or it could be recursive.

I was wondering what's the reason for such changes and if it's possible to roll them back.

I wouldn't want to roll back these changes at this point, as I think it's a necessary long term change. That said, like I have done for the other config changes I'm happy to reintroduce them for the rest of the 5.x.x cycle with a warning that they are deprecated and a link to the upgrade guide: https://timothycrosley.github.io/isort/docs/upgrade_guides/5.0.0/#-ac-wl-ws-tc-sp-sp-sl-sg-sd-rr-ot-nlb-nis-ls-le-lbt-lai-fss-fgw-ff-fass-fas-dt-ds-df-cs-ca-af-ac

If this would be helpful let me know.

Thanks!

~Timothy

[karrtikr]

Thanks, makes sense. I'll consult with the team and get back to you soon.

[karrtikr]

That said, like I have done for the other config changes I'm happy to reintroduce them for the rest of the 5.x.x cycle with a warning that they are deprecated and a link to the upgrade guide

I consulted with the team and this will do for now. We're thinking of showing a warning prompt to users passing all your warning messages. Are there any other warning messages you show regarding isort5?

[timothycrosley]

Awesome! I've pushed out the improved isort warning reporting for these options in the 5.3.0 release.

Are there any other warning messages you show regarding isort5?

From CLI usage, the following other options don't do anything in isort 5 but do provide a warning linking to the 5.0.0 upgrade guide instead of failing: "--recursive", "-rc", "--dont-skip", "-ns", "--apply", "-k", "--keep-direct-and-as".

Similarly, from config file or API usage of isort ("not_skip", "keep_direct_and_as_imports") both don't have any impact, and do output a warning linking to the 5.0.0 upgrade guide.

And that should be the extend of it.

Thanks!

~Timothy

[karrtikr]

That's great. Couple of comments,

• I just realized that users might be receiving multiple warnings messages at the same time - one for each CLI flag? + other warnings. Showing separate warning prompts for all these messages doesn't seem right, ideally we should only have one warning prompt pointing to the upgrade guide.
  Is it possible to include something like an Error code in all these deprecation messages, which will help us club all these warnings together and show just one single prompt? Or should check for string "Please see the 5.0.0 upgrade guide" suffice?

• If you have multiple configs, they will need to be merged into 1 single one.

  We're not showing warnings for users with multiple configs, I guess we actually can't. Is it possible to warn users about this kind of scenario?

[timothycrosley]

Thanks for the feedback! I've ensured a minimal number of warnings (one for each type) will be raised and that each will have a code associated with it for easy identification:

https://timothycrosley.github.io/isort/docs/warning_and_error_codes/W0500/

This is deployed to PyPI in the 5.3.1 release.

Thanks!

~Timothy

[karrtikr]

Thanks for including the codes!

Should be W0503: 6ed0cd3#r41272904

Any thoughts on the second question I had?

[timothycrosley]

Thanks for the note! I've pushed out a hot fix release to fix the incorrect code (5.3.2).

Yes, sorry meant to answer it earlier! It is possible to identify the other config files that would have been merged, but it would come with a hefty performance penalty on some code bases as it isn't trivial: isort would have to look at every one of the "shared" config file formats it is able to read from, all the way up to root, to see if any of them contain an isort section that would have been merged. Just seeing that the config files isn't enough, since they are used for many tools.

[karrtikr]

Cool, that's understandable. The number of users who fall into that category and won't receive any other warning messages are pretty low anyway, so we need not worry 🙂

Thanks again for addressing all the concerns, closing this.