Use isort

[jxlil]

Added isort to scrapy and sorted all imports.

Closes #5737, fixes #5810.

[codecov]

Codecov Report

Merging #5806 (5dcf8b9) into master (e71eab6) will increase coverage by 0.02%.
The diff coverage is 99.24%.

❗ Current head 5dcf8b9 differs from pull request most recent head 17354a6. Consider uploading reports for the commit 17354a6 to get more accurate results

@@            Coverage Diff             @@
##           master    #5806      +/-   ##
==========================================
+ Coverage   88.91%   88.93%   +0.02%     
==========================================
  Files         162      162              
  Lines       10988    10992       +4     
  Branches     1797     1798       +1     
==========================================
+ Hits         9770     9776       +6     
+ Misses        938      937       -1     
+ Partials      280      279       -1     

Impacted Files	Coverage Δ
scrapy/commands/view.py	92.30% <ø> (ø)
scrapy/core/downloader/contextfactory.py	87.50% <ø> (ø)
scrapy/core/downloader/handlers/__init__.py	92.72% <ø> (ø)
scrapy/core/downloader/handlers/http2.py	100.00% <ø> (ø)
scrapy/core/scheduler.py	93.75% <ø> (ø)
scrapy/core/spidermw.py	99.44% <ø> (ø)
scrapy/downloadermiddlewares/decompression.py	100.00% <ø> (ø)
scrapy/downloadermiddlewares/httpcache.py	93.97% <ø> (ø)
scrapy/extensions/statsmailer.py	0.00% <0.00%> (ø)
scrapy/http/headers.py	98.52% <ø> (ø)
... and 94 more

[Gallaecio]

@jxlil What do you think about making #5734 (comment) part of this pull request? Instead of adding isort to tox.ini, we could actually remove from tox.ini and CI jobs anything that we handle with pre-commit now, and have a single pre-commit CI job.

[jxlil]

@Gallaecio Sounds good. I will update this PR.

[Gallaecio]

Sounds good. I will update this PR.

I was not just suggesting that you remove the tox entry for isort, but also add the pre-commit CI job, as described in #5734 (comment).

You don‘t have to do it, we can do it in a separate, later pull request, but then please add back the CI and tox configu you had. However we do it, after we merge this pull request we really want the CI to fail if any bad import sorting is introduced in the future.

[jxlil]

I don't have much experience adding CI jobs. However I believe that this action should cause CI to fail if pre-commit makes any changes.

[Gallaecio]

Great finding!

[wRAR]

     RuntimeError: The Poetry configuration is invalid:
       - [extras.pipfile_deprecated_finder.2] 'pip-shims<=0.3.4' does not match '^[a-zA-Z-_.0-9]+$'

Uhh. Is this some problem with the pre-commit action itself?

[jxlil]

This problem is related to isort, updating to version 5.12.0 fixes the bug (PyCQA/isort#2077). However, this version supports only Python 3.8+, should I update the action to something like this?

- name: pre-commit
  uses: pre-commit/action@v3.0.0
  with:
    python-version: "3.11"

In the logs I see that pre-commit uses Python 3.10 by default.

[Gallaecio]

The problem I see with the current CI approach is that it will run pre-commit for every check job. And also, we should remove the CI jobs that are not needed once we have a pre-commit CI job.

Let me make some changes to the CI file myself…

[Gallaecio]

Done to address #5810, since otherwise we would be removing pylint from checks.yml in this pull request to re-add it when we remove pylint from pre-commit.

[Gallaecio]

Looks good to me, thanks!