Adds black and pre-commit hooks #239
Conversation
Replaces yapf as formatter
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
As black replaces " with ', which makes invalid JSON
|
Can you try to set the max line length for flake8 to 88 (black's default)? I should have some time to fix the remaining line length issues if that's ok with you |
Using YAML anchor + alias for nb
…docstyle expects it
|
Max length is now 88 and some checks are ignored. Sure you can help to fix any of them I configured pre-commit to print the flake8 warnings, but treat is as passed. |
There was a problem hiding this comment.
Very nice @sverhoeven , I think this really helps to improve the package! I fixed all line lengths issues except for in the notebooks. I did have a few minor comments, could you look at those please?
.pre-commit-config.yaml
Outdated
| additional_dependencies: [isort==5.9.3] | ||
| - id: nbqa-mypy | ||
| additional_dependencies: [mypy==0.910] | ||
| # TODO renable when erros are fixed/ignored |
There was a problem hiding this comment.
nope, https://pypi.org/project/isort/5.9.3/ and https://pypi.org/project/mypy/0.910/ are latest versions on PyPI
Versions are there to make sure the .py and .ipynb are using same versions of linters/formatters.
There was a problem hiding this comment.
I meant the #TODO which was above a block that was not disabled, but I guess we'll get to it once we addressed the remaining issues.
.pre-commit-config.yaml
Outdated
| # rev: "v2.9.6" | ||
| # hooks: | ||
| # - id: pylint | ||
| # TODO renable when erros are fixed/ignored |
There was a problem hiding this comment.
Same isort,mypi or do you mean something else then version?
| GEOGCS["GCS_WGS_1984",DATUM["D_WGS_1984",SPHEROID["WGS_1984",6378137,298.257223563]],PRIMEM["Greenwich",0],UNIT["Degree",0.017453292519943295]] |
There was a problem hiding this comment.
Is this file supposed to be formatted?
There was a problem hiding this comment.
It is a text file according to http://wiki.gis.com/wiki/index.php/Shapefile#Shapefile_projection_format_.28.prj.29
and can still read it with
In [1]: from cartopy.io import shapereader
In [2]: shape = shapereader.Reader('docs/examples/data/Rhine')
In [5]: list(shape.records())
Out[5]: [<Record: <shapely.geometry.polygon.Polygon object at 0x7fdfa3388a60>, {'HYBAS_ID': 2040023010, 'NEXT_DOWN': 0, 'NEXT_SINK': 2040023010, 'MAIN_BAS': 2040023010, 'DIST_SINK': 0.0, 'DIST_MAIN': 0.0, 'SUB_AREA': 163122.5, 'UP_AREA': 163122.5, 'PFAF_ID': 2326, 'SIDE': 'M', 'LAKE': 0, 'ENDO': 0, 'COAST': 0, 'ORDER': 1, 'SORT': 64, '_Slope_mea': 2.675870994597621, '_Slope_med': 1.3819899559021, '_Slope_std': 3.960913895155292, '_Slope_min': 0.0, '_Slope_max': 42.11079788208008, '_Slope_ran': 42.11079788208008, '_Slope_var': 0, '_Slope_v_1': 15.688838884834267, '_Urban_cou': 2617913.0, '_Urban_sum': 220178.0, '_Urban_mea': 0.084104399191264, '_Altitudem': -185.0, '_Altitud_1': 3944.0, '_Altituder': 4129.0, 'Urban_Perc': 8.41, '_variety': 0}, <fields>>]| for _ in ( | ||
| "1.5, 2.5", | ||
| [1.5, 2.5], | ||
| [1.5, 2.5], |
There was a problem hiding this comment.
Looks like it, can we fix that in a separate issue?
| - id: nbqa-mypy | ||
| additional_dependencies: [mypy==0.910] | ||
| # TODO renable when erros are fixed/ignored | ||
| - id: nbqa-flake8 |
There was a problem hiding this comment.
Could it be that the notebooks (in docs) are not picked up by these checks?
This check passes on pre-commit run --all-files for me, but if I do pip install nbqa and then nbqa flake8 docs/*.ipynb I get a list of issues.
There was a problem hiding this comment.
I think the issue is
args: *fa
, I'm not sure what it's meant to do here, but if you remove it then pre-commit run nbqa-flake8 --all-files will fail too
There was a problem hiding this comment.
The flake8 hook is verbose by default while nbqa-flake8 was not, I forced it to be verbose so now warnings show up.
The *fa is a YAML alias of YAML anchor defined in flake8 hook with @fa so I do not have to repeat the list of flake8 plugins.
There was a problem hiding this comment.
I didn't know you could do that, thanks! This'll be useful in other projects
Co-authored-by: Peter Kalverla <peter.kalverla@gmx.com>
|
SonarCloud Quality Gate failed. |
|
Thanks for reviewing |
|
Just FYI, black now has a Jupyter hook, I'd advise using that instead of |
Also formats files to be compliant.
Fixes #111
Fixes #241
Tried to add flake8, but it complains a lot and all complaints have to manually fixed.
Do we want to be flake8 compliant and drop prospector?Yep we do warnings first and once fixed fatal errors.