Restore isort to pre-commit check; apply to entire codebase #857

StanczakDominik · 2020-06-20T08:22:43Z

#846 removed isort from the pre-commit suite because it didn't play nicely
with CI. Well, according to
PyCQA/isort#1147 (comment)
isort will work well now. Let's see if that works.

This should probably not get merged yet; let's give isort a few days to put
a release out.

I also brought back the 88 line limit for consistency between the two
(it got the suite stuck in a permanent 2-cycle of 2 character line length
changes) - it turns out it's the default
in black, and I figure if Łukasz Langa believes in that default,
it's a sane default.

I also added a file that lets you ignore isort/black commits in git blame locally.

make sure the commit hash for this PR is correct upon squashing; this will take a manual local merge.

codecov · 2020-06-20T08:25:08Z

Codecov Report

Merging #857 into master will not change coverage.
The diff coverage is 95.45%.

@@           Coverage Diff           @@
##           master     #857   +/-   ##
=======================================
  Coverage   96.18%   96.18%           
=======================================
  Files          60       60           
  Lines        5368     5368           
=======================================
  Hits         5163     5163           
  Misses        205      205

Impacted Files	Coverage Δ
plasmapy/formulary/parameters.py	`99.00% <ø> (ø)`
plasmapy/particles/elements.py	`100.00% <ø> (ø)`
plasmapy/particles/particle_class.py	`96.57% <ø> (ø)`
plasmapy/simulation/particletracker.py	`98.30% <ø> (ø)`
plasmapy/utils/error_messages.py	`89.23% <ø> (ø)`
plasmapy/utils/roman/__init__.py	`100.00% <ø> (ø)`
plasmapy/__init__.py	`34.48% <50.00%> (ø)`
plasmapy/diagnostics/thomson.py	`100.00% <100.00%> (ø)`
plasmapy/formulary/__init__.py	`100.00% <100.00%> (ø)`
plasmapy/formulary/braginskii.py	`99.72% <100.00%> (ø)`
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a4e69a9...cd0af01. Read the comment docs.

StanczakDominik · 2020-06-23T19:02:21Z

plasmapy/diagnostics/langmuir.py

 import numpy as np
 from astropy import units as u
+from astropy.constants import si as const
 from astropy.visualization import quantity_support
-from scipy.optimize import curve_fit
-
 from plasmapy.particles import Particle
 from plasmapy.utils.decorators import validate_quantities
+from scipy.optimize import curve_fit


More tinkering with config

separate plasmapy imports by newline

StanczakDominik · 2020-07-25T07:59:16Z

I'll have to try this again now that isort 5 is out

StanczakDominik · 2020-07-31T08:18:21Z

Okay. I updated isort to 5.2.2 (the current stable release), merged in current master and #878 because that's soon to be merged, then applied the full pre-commit suite to all of those changes. The commit to really look through here would thus be 18b60ba . I think the changes it applies look fine now.

StanczakDominik · 2020-07-31T08:19:26Z

plasmapy/__init__.py

-    simulation,
-    utils,
-)
+from plasmapy import diagnostics, formulary, particles, plasma, simulation, utils


This looks a bit weird but I guess it's just below the line length limit, and I'm not going to argue with autotools on that point.

Agreed! I guess it's practice in bowing down to our robotic overlords.

StanczakDominik · 2020-07-31T16:21:10Z

Oh right, squash merges do that.

I'll rebase later.

StanczakDominik · 2020-08-01T09:19:40Z

Okay, fixed the conflicts!

rocco8773 · 2020-08-03T00:34:25Z

setup.cfg

 [isort]
 # Set sorting of imports to be consistent with black formatting
-line_length=90
 multi_line_output=3
-include_trailing_comma: True
+include_trailing_comma=True
+force_grid_wrap=0
+use_parentheses=True
+line_length=88


After playing around with isort, I'm really excited to have this implemented!!

According to isort, the preferred configuration files are .isort.cfg or pyproject.toml. Since we already have a pyproject.toml, I say we migrate the configuration to that file.

After playing with the isort options, I think this would be the closes to what we've been implementing so far...

[tool.isort] line_length = 88 wrap_length = 80 sections = ["FUTURE", "STDLIB", "FIRSTPARTY", "LOCALFOLDER"] known_first_party = ["plasmapy", ] default_section = "STDLIB" multi_line_output = 3 use_parentheses = true include_trailing_comma = true force_alphabetical_sort_within_sections = true lines_between_types = 1

Note, this will force third party packages and standard library packages into the same import section. That is how we've been organizing imports thus far. If we want to split up standard library and third party imports, then that's fine too. I just figure we stick with what we've been doing.

I don't really see that as something worth setting up custom settings for - I'd be happy to go for isort defaults. But since you've gone to the trouble of doing it, and you likely have Opinions about this, I'll add this config and rerun the full isort pass.

StanczakDominik · 2020-08-03T05:39:55Z

@rocco8773 I've applied the changes. If they look okay, hit approve - but please don't merge, I'll do it manually locally to get .git-blame-ignore-revs right in a single pass.

Apply Erik's changes to isort config Move its config to pyproject.toml Apply isort with the new config to the entire codebase Create .git-blame-ignore-revs

rocco8773 · 2020-08-03T19:11:33Z

plasmapy/particles/tests/test_parsing.py

@@ -6,8 +6,7 @@
    InvalidElementError,
    InvalidParticleError,
 )
-from plasmapy.particles.parsing import (
-    # duplicate with utils.pytest_helpers.error_messages.call_string?
+from plasmapy.particles.parsing import (  # duplicate with utils.pytest_helpers.error_messages.call_string?


This is odd. I've confirmed isort is placing the comment here, which then violates PEP8 max line-length. I've played with the configuration options, but I couldn't find a solution. Maybe run black after isort.

In the end, this comment does not make much sense to me and can probably be deleted.

Okay, I'll remove that one. Anything else that needs changing?

Nope, everything else looks good to me.

StanczakDominik marked this pull request as draft June 20, 2020 08:29

StanczakDominik commented Jun 23, 2020

View reviewed changes

StanczakDominik requested a review from rocco8773 July 31, 2020 08:15

StanczakDominik marked this pull request as ready for review July 31, 2020 08:15

StanczakDominik commented Jul 31, 2020

View reviewed changes

rocco8773 mentioned this pull request Jul 31, 2020

Improve Package Exposure in Documentaiton #878

Merged

StanczakDominik force-pushed the isort-returns branch from 51bf085 to 24c4518 Compare August 1, 2020 09:19

rocco8773 reviewed Aug 3, 2020

View reviewed changes

StanczakDominik added 2 commits August 3, 2020 11:27

Bring back isort to pre-commit suite

9602b5b

Apply Erik's changes to isort config Move its config to pyproject.toml Apply isort with the new config to the entire codebase Create .git-blame-ignore-revs

Adjust git-blame-ignore-revs

cd0af01

rocco8773 mentioned this pull request Aug 3, 2020

Decompose plasmapy/utils/roman #883

Merged

rocco8773 reviewed Aug 3, 2020

View reviewed changes

rocco8773 approved these changes Aug 3, 2020

View reviewed changes

StanczakDominik force-pushed the isort-returns branch from c1b73e9 to cd0af01 Compare August 4, 2020 09:22

StanczakDominik merged commit cd0af01 into PlasmaPy:master Aug 4, 2020

StanczakDominik deleted the isort-returns branch August 4, 2020 10:00

namurphy added the linters Code linters and autoformatters label May 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Restore isort to pre-commit check; apply to entire codebase #857

Restore isort to pre-commit check; apply to entire codebase #857

StanczakDominik commented Jun 20, 2020 •

edited

Loading

codecov bot commented Jun 20, 2020 •

edited

Loading

StanczakDominik Jun 23, 2020

StanczakDominik commented Jul 25, 2020

StanczakDominik commented Jul 31, 2020

StanczakDominik Jul 31, 2020

rocco8773 Jul 31, 2020

StanczakDominik commented Jul 31, 2020

StanczakDominik commented Aug 1, 2020

rocco8773 Aug 3, 2020 •

edited

Loading

StanczakDominik Aug 3, 2020

StanczakDominik commented Aug 3, 2020

rocco8773 Aug 3, 2020

StanczakDominik Aug 3, 2020

rocco8773 Aug 3, 2020

Restore isort to pre-commit check; apply to entire codebase #857

Restore isort to pre-commit check; apply to entire codebase #857

Conversation

StanczakDominik commented Jun 20, 2020 • edited Loading

codecov bot commented Jun 20, 2020 • edited Loading

Codecov Report

StanczakDominik Jun 23, 2020

Choose a reason for hiding this comment

StanczakDominik commented Jul 25, 2020

StanczakDominik commented Jul 31, 2020

StanczakDominik Jul 31, 2020

Choose a reason for hiding this comment

rocco8773 Jul 31, 2020

Choose a reason for hiding this comment

StanczakDominik commented Jul 31, 2020

StanczakDominik commented Aug 1, 2020

rocco8773 Aug 3, 2020 • edited Loading

Choose a reason for hiding this comment

StanczakDominik Aug 3, 2020

Choose a reason for hiding this comment

StanczakDominik commented Aug 3, 2020

rocco8773 Aug 3, 2020

Choose a reason for hiding this comment

StanczakDominik Aug 3, 2020

Choose a reason for hiding this comment

rocco8773 Aug 3, 2020

Choose a reason for hiding this comment

StanczakDominik commented Jun 20, 2020 •

edited

Loading

codecov bot commented Jun 20, 2020 •

edited

Loading

rocco8773 Aug 3, 2020 •

edited

Loading