Update to isort 5.7

[Anthchirp]

I followed the setup instructions, and the default pre-commit hook fails as follows:

$ git commit --allow-empty

=============================================== FAILURES ===============================================
______________________________________________ test_isort ______________________________________________

    def test_isort() -> None:
        ''' Assures that the Python codebase imports are correctly sorted.
    
        '''
        chdir(TOP_PATH)
        proc = run(["isort", "-df", "-rc", "-c", *ls_files("*.py")], capture_output=True)
>       assert proc.returncode == 0, f"isort issues:\n{proc.stdout.decode('utf-8')}"
E       AssertionError: isort issues:
E         
E       assert 2 == 0
E        +  where 2 = CompletedProcess(args=['isort', '-df', '-rc', '-c', '_setup_support.py', 'bokeh/__init__.py', 'bokeh/__main__.py', 'bo...config] [--honor-noqa]\n             [files [files ...]]\nisort: error: argument -f/--future: expected one argument\n').returncode

tests/codebase/test_isort.py:35: AssertionError
======================================= short test summary info ========================================
FAILED tests/codebase/test_isort.py::test_isort - AssertionError: isort issues:
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! KeyboardInterrupt !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
/home/markus/bokeh/miniconda/envs/bkdev/lib/python3.8/subprocess.py:1011: KeyboardInterrupt
(to show a full traceback on KeyboardInterrupt use --full-trace)
===================================== 1 failed, 3 passed in 49.25s =====================================
^C

This seems to be caused by isort 5 having changed its command line flags, and the test expecting isort v4.

I'd recommend the following changes

isort 4	isort 5	recommended action
-df --diff	--df --diff	change currently used -df to universally accepted --diff
-rc --recursive	no equivalent	remove -rc flag, looks like we pass individual files anyway.

[bryevdv]

Yes isort 5 came out last week and broke CI at my work, I am actually surprised we have not also seen CI issues. Those changes look good to me.

[Anthchirp]

Turns out this is not as simple as just changing

diff --git a/tests/codebase/test_isort.py b/tests/codebase/test_isort.py
index d2aadb305..f3b26ee61 100644
--- a/tests/codebase/test_isort.py
+++ b/tests/codebase/test_isort.py
@@ -31,7 +31,7 @@ def test_isort() -> None:
 
     '''
     chdir(TOP_PATH)
-    proc = run(["isort", "-df", "-rc", "-c", *ls_files("*.py")], capture_output=True)
+    proc = run(["isort", "--diff", "-c", *ls_files("*.py")], capture_output=True)
     assert proc.returncode == 0, f"isort issues:\n{proc.stdout.decode('utf-8')}"
 
 #-----------------------------------------------------------------------------

as there are other major differences in how isort works.

For example isort v5 will try to make the following changes:

-        from bokeh.models import Tabs, Panel
+        from bokeh.models import Panel, Tabs

...

-# Bokeh imports
-import bokeh.util.version as buv
from bokeh import __version__
from bokeh.models import Model
from bokeh.resources import _get_cdn_urls
+# Bokeh imports
+from bokeh.util import version as buv

etc., so quite invasive.

It is therefore probably not possible to support versions 4 and 5 side-by-side.
I think it'd probably make more sense to revisit setting up pre-commit (#9439), which could ensure everyone runs the exact same version with the same settings.

[bryevdv]

@Anthchirp We definitely don't need to support both isort version 4 and version 5. If isort 5 is proposing lots of spurious changes, and can't be configured to behave like the old version, then I am fine just pinning the version to <5 in the conda env files at least for now. The environment files are what ensures everyone runs the expected versions of things for Bokeh dev.

[bryevdv]

I would like to understand why CI is passing, and the pre-commit hook fails, because they both run literally the exact same test_isort.py test, but I don't see the isort version pinned in the CI env files.

[Anthchirp]

The relevant ci test environment specifies

bokeh/ci/environment-test-3.7.yml

Lines 2 to 4 in bab7326

	channels:
	- defaults
	- conda-forge

whereas the development environment says

bokeh/environment.yml

Lines 2 to 3 in bab7326

	channels:
	- conda-forge

the conda-forge channel has isort 5.0.9, whereas default=pkgs/main/linux-64 is still with 4.3.21

[bryevdv]

Interesting, I think maybe the dev env file should be updated to match the CI one.

As for isort: If there is a configuration for isort 5 that works without us having to change any current imports, I am 👍 to change the env files to specify isort >=5, and updating the test script. If there is not then I would suggest we pin the env to isort < 5.

Thoughts?

[Anthchirp]

Will have a look and see if I can get isort 5 to work 👍

[bryevdv]

I guess I should add: some small changes would be fine but there's currently alot of noise, e.g. adding import section header to imports inside functions. I have to believe there is a way to turn that off. FYI note we have an isort section in the top level setup.cfg think some of the options are now obsolete but that's also where config changes could go.

[bryevdv]

note that defaults finally picked up isort 5. I have pinned it <5 in #10308 for now

[bryevdv]

PyCQA/isort#1604 was closed and is in isort 5.7 so we should be able to update now