Correct/clarify message for wrong-import-order #9236

azinneck0485 · 2023-11-14T01:52:27Z

Type of Changes

	Type
✓	🐛 Bug fix

Description

Adds clarification in the message for the wrong-import-order check.

The original message was confusing, especially when multiple imports were on a single line, because the entire import statement is listed twice in the message ("import X, Y, Z" should be placed before "import X, Y, Z").

Changes were made to extract the "current" import name and the "out of order" import names, and separate them by "isort order", so the user is able to clearly see what changes need to be made to address the problem. For example, standard import "X" should be placed before third order import "Y" and first order import "Z".

No functional tests were added, but the output for the wrong-import-order test was updated to reflect the new messages produced by the change. The code has passed the full tox test suite as well as all primer tests and a documentation fragment added to explain the changes.

Closes #8808

codecov · 2023-11-14T02:04:16Z

Codecov Report

Merging #9236 (8bff674) into main (6f83d5a) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9236      +/-   ##
==========================================
+ Coverage   95.79%   95.81%   +0.01%     
==========================================
  Files         173      173              
  Lines       18722    18761      +39     
==========================================
+ Hits        17935    17975      +40     
+ Misses        787      786       -1

Files	Coverage Δ
pylint/checkers/imports.py	`94.46% <100.00%> (+0.66%)`	⬆️
pylint/constants.py	`100.00% <100.00%> (ø)`

Pierre-Sassoulas

This look great already, the primer in particular looks very promising. Just some nits :)

pylint/checkers/imports.py

Pierre-Sassoulas

Thank you for the changes. Sorry I missed that we could also use fstrings in the 3 args (it's faster than string addition by a lot).

pylint/checkers/imports.py

azinneck0485 · 2023-11-15T04:19:57Z

Looks like there are a few things I should add or change in the test to improve code coverage. I'll work on that over the next day or so and push a commit with those updates.

Pierre-Sassoulas

Look amazing, the message is sometime long, let me know what you think of the proposal.

pylint/checkers/imports.py

Pierre-Sassoulas · 2023-11-26T09:59:28Z

Classifying as enhancement and delaying to 3.1.0 because there was a lot more done than simply fixing the original bug.

Improve the message provided for wrong-import-order check. Instead of the import statement ("import x"), the message now specifies the import that is out of order and which imports should come after it. As reported in the issue, this is particularly helpful if there are multiple imports on a single line that do not follow the PEP8 convention. The message will report imports as follows: - For "import X", it will report "(standard/third party/first party/local) import X" - For "import X.Y" and "from X import Y", it will report "(standard/third party/first party/local) import X.Y" The import category is specified to provide explanation as to why pylint has issued the message and guidence to the developer on how to fix the problem. Closes pylint-dev#8808 Co-authored-by: Pierre Sassoulas <[email protected]>

New sphinx now check that a block of code is valid.

Improve the message provided for wrong-import-order check. Instead of the import statement ("import x"), the message now specifies the import that is out of order and which imports should come after it. As reported in the issue, this is particularly helpful if there are multiple imports on a single line that do not follow the PEP8 convention. The message will report imports as follows: - For "import X", it will report "(standard/third party/first party/local) import X" - For "import X.Y" and "from X import Y", it will report "(standard/third party/first party/local) import X.Y" The import category is specified to provide explanation as to why pylint has issued the message and guidence to the developer on how to fix the problem. Closes #8808 Co-authored-by: Pierre Sassoulas <[email protected]>

github-actions · 2023-11-26T11:09:29Z

🤖 Effect of this PR on checked open source code: 🤖

Effect on home-assistant:
The following messages are now emitted:

import-private-name:
Imported private module (_collections_abc)
https://github.com/home-assistant/core/blob/97d38dae09ea69ea60f164a84740f978417e9d7f/homeassistant/components/renson/button.py#L6

Effect on pygame:
The following messages are now emitted:

wrong-import-order:
standard import "copyreg" should be placed before third party imports "pygame.base.", "pygame.constants.", "pygame.version." (...) "pygame.color", "pygame.bufferproxy", "pygame.math"*
https://github.com/pygame/pygame/blob/653dd7132362f4665a1f508d3af0f3c8cf6ff142/src_py/__init__.py#L308
wrong-import-order:
standard import "time" should be placed before third party imports "numpy", "cv2"
https://github.com/pygame/pygame/blob/653dd7132362f4665a1f508d3af0f3c8cf6ff142/src_py/_camera_opencv.py#L10
wrong-import-order:
standard import "warnings" should be placed before third party imports "pygame.pixelcopy.array_to_surface", "numpy", "numpy.array"
https://github.com/pygame/pygame/blob/653dd7132362f4665a1f508d3af0f3c8cf6ff142/src_py/surfarray.py#L55
wrong-import-order:
standard import "warnings" should be placed before third party imports "pygame.mixer", "numpy"
https://github.com/pygame/pygame/blob/653dd7132362f4665a1f508d3af0f3c8cf6ff142/src_py/sndarray.py#L43