Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add isort #63

Closed
wants to merge 1 commit into from
Closed

Add isort #63

wants to merge 1 commit into from

Conversation

jatkinson1000
Copy link
Owner

Add isort as a dependency to pyproject.toml and include isort in the linting workflow.

Apply isort to code.

Add badge to main page.

@jatkinson1000 jatkinson1000 self-assigned this Feb 9, 2024
@jatkinson1000 jatkinson1000 added documentation Improvements or additions to documentation enhancement New feature or request labels Feb 9, 2024
Copy link

codecov bot commented Feb 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (78ca54b) 95.45% compared to head (32f03b7) 95.45%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main      #63   +/-   ##
=======================================
  Coverage   95.45%   95.45%           
=======================================
  Files          24       24           
  Lines        1321     1321           
=======================================
  Hits         1261     1261           
  Misses         60       60           

@jatkinson1000
Copy link
Owner Author

black does some sorting of imports already and can conflict with isort (see here) so closing this PR for now.

@jatkinson1000 jatkinson1000 deleted the isort branch February 9, 2024 14:59
@LiamPattinson
Copy link
Contributor

You can to set isort to run in black mode by adding this to your pyproject.toml:

[tool.isort]
profile = "black"

@jatkinson1000
Copy link
Owner Author

Yeah, I saw that, but my bigger issue is that is disagrees with pylint on import ordering, so I don't think it really gains me anything.
Perhaps a motivation to move towards a single unifying tool i.e. ruff...

@LiamPattinson
Copy link
Contributor

Oh right, I tend not to use pylint because of its tendency to complain about things I think are fairly reasonable. I hadn't tried using it alongside isort, but I'm not surprised there's a conflict. Ruff can take the roles of black, isort and flake8, but to my knowledge you still need to do some set up to get the isort and black parts to agree and it can't perform some of the more in-depth checks of pylint.

@jatkinson1000
Copy link
Owner Author

Yeah, this is why I haven't transitioned thus far, but it is very distractedboyfriend.png

@TomHall2020
Copy link
Contributor

Yeah am a big fan of ruff.
On my machine pylint on archery utils takes over 10s and ruff is pretty much instant...

 ~/dev/forks/archeryutils
❯ time pylint archeryutils

--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)

pylint archeryutils  9.61s user 0.49s system 95% cpu 10.570 total

 ~/dev/forks/archeryutils
❯ time ruff archeryutils
ruff archeryutils  0.01s user 0.02s system 32% cpu 0.097 total

@jatkinson1000
Copy link
Owner Author

Yeah, I'm tempted, the last thing holding me back is the fact it is not quite as picky as pylint, though if I get some time (hahaha) I might have a play and see if it covers everything I have opinions about.

I also discovered astral's uv repo today so am exploring that.

@TomHall2020
Copy link
Contributor

TomHall2020 commented Feb 20, 2024

I belive you can configure ruff to run the pylint ruleset, and a quick test on my working branch at the moment shows it being even_more_picky

❯ pylint archeryutils
************* Module archeryutils.handicaps.tests.test_handicaps
archeryutils/handicaps/tests/test_handicaps.py:1:0: C0302: Too many lines in module (1074/1000) (too-many-lines)
archeryutils/handicaps/tests/test_handicaps.py:380:4: R0913: Too many arguments (6/5) (too-many-arguments)
archeryutils/handicaps/tests/test_handicaps.py:380:61: W0613: Unused argument 'target' (unused-argument)
archeryutils/handicaps/tests/test_handicaps.py:395:4: R0913: Too many arguments (6/5) (too-many-arguments)
archeryutils/handicaps/tests/test_handicaps.py:395:60: W0613: Unused argument 'hc_dat' (unused-argument)

------------------------------------------------------------------
Your code has been rated at 9.97/10 (previous run: 9.89/10, +0.08)

❯ ruff --extend-select PL archeryutils
archeryutils/classifications/agb_outdoor_classifications.py:194:19: PLR2004 Magic value used in comparison, consider replacing `3` with a constant variable
archeryutils/classifications/classification_utils.py:270:50: PLR2004 Magic value used in comparison, consider replacing `3` with a constant variable
archeryutils/classifications/classification_utils.py:274:50: PLR2004 Magic value used in comparison, consider replacing `3` with a constant variable
archeryutils/classifications/tests/test_agb_field.py:300:9: PLR0913 Too many arguments in function definition (6 > 5)
archeryutils/classifications/tests/test_agb_outdoor.py:432:9: PLR0913 Too many arguments in function definition (6 > 5)
archeryutils/classifications/tests/test_agb_outdoor.py:529:9: PLR0913 Too many arguments in function definition (6 > 5)
archeryutils/handicaps/handicap_equations.py:455:9: PLR5501 [*] Use `elif` instead of `else` then `if`, to reduce indentation
archeryutils/handicaps/handicap_equations.py:458:13: PLR5501 [*] Use `elif` instead of `else` then `if`, to reduce indentation
archeryutils/handicaps/handicap_equations.py:535:5: PLR0913 Too many arguments in function definition (6 > 5)
archeryutils/handicaps/handicap_functions.py:42:5: PLR0913 Too many arguments in function definition (6 > 5)
archeryutils/handicaps/handicap_functions.py:222:5: PLR0912 Too many branches (18 > 12)
archeryutils/handicaps/handicap_functions.py:222:5: PLR0915 Too many statements (65 > 50)
archeryutils/handicaps/handicap_functions.py:343:9: PLR5501 [*] Use `elif` instead of `else` then `if`, to reduce indentation
archeryutils/handicaps/handicap_functions.py:355:5: PLR0913 Too many arguments in function definition (6 > 5)
archeryutils/handicaps/handicap_functions.py:413:5: PLR0913 Too many arguments in function definition (11 > 5)
archeryutils/handicaps/tests/test_handicaps.py:137:23: PLR2004 Magic value used in comparison, consider replacing `2.0` with a constant variable
archeryutils/handicaps/tests/test_handicaps.py:139:23: PLR2004 Magic value used in comparison, consider replacing `2.0` with a constant variable
archeryutils/handicaps/tests/test_handicaps.py:141:23: PLR2004 Magic value used in comparison, consider replacing `3.0` with a constant variable
archeryutils/handicaps/tests/test_handicaps.py:378:30: PLR2004 Magic value used in comparison, consider replacing `3` with a constant variable
archeryutils/handicaps/tests/test_handicaps.py:380:9: PLR0913 Too many arguments in function definition (6 > 5)
archeryutils/handicaps/tests/test_handicaps.py:395:9: PLR0913 Too many arguments in function definition (6 > 5)
archeryutils/rounds.py:50:9: PLR0913 Too many arguments in function definition (6 > 5)
archeryutils/rounds.py:204:9: PLR0913 Too many arguments in function definition (6 > 5)
archeryutils/targets.py:327:9: PLR0912 Too many branches (13 > 12)
archeryutils/tests/test_rounds.py:41:38: PLR2004 Magic value used in comparison, consider replacing `36` with a constant variable
archeryutils/tests/test_rounds.py:49:38: PLR2004 Magic value used in comparison, consider replacing `36` with a constant variable
archeryutils/tests/test_rounds.py:128:38: PLR2004 Magic value used in comparison, consider replacing `36` with a constant variable
archeryutils/tests/test_rounds.py:135:38: PLR2004 Magic value used in comparison, consider replacing `50.0` with a constant variable
archeryutils/tests/test_rounds.py:137:38: PLR2004 Magic value used in comparison, consider replacing `1.22` with a constant variable
archeryutils/tests/test_rounds.py:291:42: PLR2004 Magic value used in comparison, consider replacing `2700` with a constant variable
archeryutils/tests/test_rounds.py:339:45: PLR2004 Magic value used in comparison, consider replacing `100` with a constant variable
archeryutils/tests/test_rounds.py:350:45: PLR2004 Magic value used in comparison, consider replacing `75` with a constant variable
archeryutils/tests/test_targets.py:154:35: PLR2004 Magic value used in comparison, consider replacing `0.04` with a constant variable
archeryutils/tests/test_targets.py:330:35: PLR2004 Magic value used in comparison, consider replacing `0.8` with a constant variable
archeryutils/tests/test_targets.py:382:38: PLR2004 Magic value used in comparison, consider replacing `11` with a constant variable
archeryutils/tests/test_targets.py:389:38: PLR2004 Magic value used in comparison, consider replacing `6` with a constant variable
Found 36 errors.

Not that I want to sign us up to having to name all the variables we test against, so some more refined rule selection would be called for here.

Can configure the rule selection via pyproject.toml to enforce consistency of what is being linted for.

@jatkinson1000
Copy link
Owner Author

jatkinson1000 commented Feb 20, 2024

👀
In that case I will spend some time playing in my current projects and consider a move.
Though some of those magic number flags in the tests are arguably a little too picky 😅

@TomHall2020
Copy link
Contributor

Would be possible to configure it to ignore that error for all test files, something like this in the config:

[tool.ruff.lint]
extend-select = "PL"

# ignore magic number comparison in test files
[tool.ruff.lint.per-file-ignores]
"**/test*.py" = ["PLR2004"]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants