Use pre commit #407

s-weigand · 2020-09-01T15:01:52Z

This PR adds a config for pre-commit, which will be used as more extensive code quality checker.
New code quality features, run before each commit on the staged files:

Autoformatting with black
Import sorting with isort
Removal of unnecessary # noqa comments with yesqa
Syntax uprades with pyupgrade
Code style checking with flake8
Checking of proper format for *.rst files with rstcheck and rst-backticks
Basic spellchecking with codespell
Some sanity check from pre-commit-hooks (We can't use check-yaml, since it doesn't support custom flags like !tuple)

All those checks will also be run by the CI on the entire code base (pre-commit run --all).

The enforcing of proper docstrings with pydocstyle is deactivated right now.
Because there are 421 isuues to be solved, which else would break the CI.
I think the best approach will be to improve the docstring quality file by file.
To check a single file, run the following command.

pydocstyle.exe <file-folder-path> --ignore-decorators=wrap_func_as_method

The --ignore-decorators=wrap_func_as_method is needed since wrap_func_as_method, generates docstrings using f-strings, which pydocstyle doesn't like

Testing

Passing the tests is mandatory.

Closing issues

closes #388 closes #389

See https://git-scm.com/docs/git-blame#Documentation/git-blame.txt---ignore-revltrevgt

This is a temporary fix until the issue is resolved, see PyCQA/pycodestyle#914

CONTRIBUTING.rst:60: (INFO/1) Hyperlink target "get-started" is not referenced. docs/source/introduction.rst:9: (WARNING/2) Inline emphasis start-string without end-string.

CONTRIBUTING.rst:63:Ready to contribute? Here's how to set up `pyglotaran` for local development. CONTRIBUTING.rst:65:1. Fork the `pyglotaran` repo on GitHub. CONTRIBUTING.rst:118: Check your Github Actions `https://github.com/<your_name_here>/pyglotaran/actions` HISTORY.rst:11:* Package was renamed to `pyglotaran` on PyPi HISTORY.rst:16:* Changed `nan_policiy` to `omit` docs/source/index.rst:4: contain the root `toctree` directive. docs/source/quickstart.rst:23:Like all data in `glotaran`, the dataset is a :class:`xarray:xarray.Dataset`. docs/source/quickstart.rst:24:You can find more infomation about the `xarray` library the `xarray hompage`_. docs/source/quickstart.rst:72:To analyze our data, we need to create a model. Create a file called `model.py` docs/source/quickstart.rst:150:You can check your model for problems with `model.validate`. docs/source/quickstart.rst:156:Now define some starting parameters. Create a file called `parameter.yml` with docs/source/quickstart.rst:195:You can `model.validate` also to check for missing parameters. docs/source/quickstart.rst:222:You can get the resulting data for your dataset with `result.get_dataset`.

glotaran/model/util.py:21: orginal ==> original glotaran/model/util.py:23: orginal ==> original glotaran/model/util.py:25: documenation ==> documentation glotaran/model/util.py:25: orginal ==> original glotaran/model/util.py:25: documenation ==> documentation glotaran/builtin/models/kinetic_image/initial_concentration.py:1: intial ==> initial glotaran/parameter/parameter.py:31: paramter ==> parameter glotaran/parameter/parameter.py:80: dictonary ==> dictionary docs/source/installation.rst:79: preffered ==> preferred glotaran/analysis/simulation.py:35: Conditionaly ==> Conditionally glotaran/analysis/result.py:36: dictonary ==> dictionary glotaran/builtin/models/kinetic_spectrum/spectral_constraints.py:80: documention ==> documentation glotaran/cli/commands/optimize.py:15: infered ==> inferred glotaran/cli/commands/optimize.py:112: occured ==> occurred glotaran/cli/commands/optimize.py:120: successfull ==> successful glotaran/cli/commands/optimize.py:120: follwing ==> following glotaran/cli/commands/optimize.py:124: occured ==> occurred glotaran/analysis/nnls.py:1: conditionaly ==> conditionally glotaran/analysis/nnls.py:13: conditionaly ==> conditionally glotaran/parameter/parameter_group.py:29: hirachy ==> hierarchy glotaran/parameter/parameter_group.py:373: seperator ==> separator glotaran/parameter/parameter_group.py:382: seperator ==> separator glotaran/parameter/parameter_group.py:383: seperator ==> separator glotaran/parameter/parameter_group.py:386: seperator ==> separator glotaran/parameter/parameter_group.py:390: seperator ==> separator glotaran/parameter/parameter_group.py:390: seperator ==> separator glotaran/parameter/parameter_group.py:404: seperator ==> separator glotaran/analysis/variable_projection.py:1: conditionaly ==> conditionally glotaran/analysis/variable_projection.py:13: conditionaly ==> conditionally glotaran/model/model.py:152: Conditionaly ==> Conditionally glotaran/model/model.py:156: standart ==> standard glotaran/model/model.py:186: dictonary ==> dictionary glotaran/model/model.py:221: dictonary ==> dictionary glotaran/cli/commands/explore.py:114: ouput ==> output docs/source/quickstart.rst:24: infomation ==> information

This was needed since black replaced the single quotes with double quotes

pydocstyle is very strict on how a good docstring should look like and also complains about missing docstrings. Since pre-commit, with all its hooks, is our new code quality checker the 421 issues pydocstyles finds would break the CI. Thus I leave the fixing of the docstrings to our core developers who know exactly what which function does and what the docstring should look like ;)

codecov · 2020-09-01T15:05:29Z

Codecov Report

Merging #407 into master will increase coverage by 0.4%.
The diff coverage is 65.5%.

@@           Coverage Diff            @@
##           master    #407     +/-   ##
========================================
+ Coverage    66.6%   67.0%   +0.4%     
========================================
  Files          65      65             
  Lines        3019    3044     +25     
  Branches      603     597      -6     
========================================
+ Hits         2011    2040     +29     
+ Misses        884     880      -4     
  Partials      124     124

Impacted Files	Coverage Δ
glotaran/builtin/models/doas/oscillation.py	`100.0% <ø> (ø)`
glotaran/cli/commands/explore.py	`0.0% <0.0%> (ø)`
glotaran/cli/commands/optimize.py	`0.0% <0.0%> (ø)`
glotaran/cli/commands/util.py	`0.0% <0.0%> (ø)`
glotaran/cli/commands/validate.py	`0.0% <0.0%> (ø)`
glotaran/cli/main.py	`0.0% <0.0%> (ø)`
glotaran/examples/__init__.py	`0.0% <0.0%> (ø)`
glotaran/examples/sequential.py	`0.0% <0.0%> (ø)`
...ile_formats/ascii/wavelength_time_explicit_file.py	`22.8% <22.5%> (+0.4%)`	⬆️
glotaran/analysis/scheme.py	`56.5% <37.5%> (+0.4%)`	⬆️
... and 51 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 c70981d...92462b5. Read the comment docs.

s-weigand · 2020-09-01T15:13:51Z

Should I fix the issues with SonarCloud Code Analysis and Codacy or @jsnel do you want to do it for the whole code base at a later point?
Btw not my errors, but it got picked up because the reformatting touched a lot of files, which weren't checked before 😉

jsnel · 2020-09-01T18:28:10Z

@s-weigand feel free to make some corrections, not everything is fixable now but do what you can.

so isort will import it after the third party imports

jsnel

Reviewed ok!

I went through the individual commits and ignored (most) of the commits tagged IGNORE which were just reformatting.

docs/source/conf.py

jsnel · 2020-09-02T21:49:18Z

docs/source/introduction.rst

@@ -6,7 +6,7 @@ It is designed to provide a state of the art modeling toolbox to researchers, in

 Its features are:

-* user-friendly modeling with a custom YAML (*.yml) based modeling language
+* user-friendly modeling with a custom YAML (``*.yml``) based modeling language


I have seen a mix of .yml and .yaml, I know both are possible, and that the creator(s) of YAML prefer .yaml whereas the rest of the world prefers .yml but better check if we don't mix and match ourselves, that may be confusing for first time users.

IMHO that is up to a different PR + issue, just for checking the text in the docs.

docs/source/quickstart.rst

glotaran/analysis/nnls.py

glotaran/__init__.py

jsnel · 2020-09-02T21:59:06Z

glotaran/__init__.py


-__version__ = '0.1.0'
+__version__ = "0.1.0"


reminder to maintainers: to be increased to 0.2.0 in the near future

glotaran/analysis/result.py

forcing import to be each on its own line

* Removed unneeded path setup * Replaced hardcoded author and title with variables * Replaced unsave http links with https * Fixed sonarcloud issues in wavelength_time_explicit_file * Made errors raised in simulate less generic * Made errors raised in IrfMultiGaussian less generic * Made errors raised in _calculate_for_k_matrix less generic * Made errors raised in Scheme less generic and combined nested if statements * Renamed set to model_set, not to overwrite builtin set * Made errors raised in Model less generic * Made errors raised in Parameter less generic * Made errors raised in ParameterGroup less generic * Made errors raised in model_attribute less generic * Made errors raised in model.decorator less generic and combined nested if * Improved some docstrings * Commented out unused variable and added TODO comment

s-weigand · 2020-09-02T23:56:28Z

Got the sonarcloud code smells down from 53 to 25 (some were caused by reformatting i.e. func("foo " "bar")).
The one Security Hotspot, is because of http://glotaran.org, but since the certificate isn't validated (i.e. by letsencrypt), I left it to be http instead of https, so users won't be prompted the "Continue on your own risk" dialog.
IMHO the other issues need to be addressed in a different PR and I feel pretty much done here, so feel free to merge it whenever you feel like it. 😄

This can be done automatically by using 'pre-commit autoupdate'

joernweissenborn · 2020-09-04T11:47:36Z

glotaran/analysis/result.py

@@ -49,7 +49,7 @@ def __init__(self,
        self._optimized_parameter = parameter
        self._nfev = nfev
        self._nvars = nvars
-        self._ndata = ndata,
+        self._ndata = (ndata,)


@s-weigand Why that?

This is just a more obious to declare a single value tuple, since a comma can be easily overlooked.

joernweissenborn · 2020-09-04T11:50:11Z

glotaran/analysis/scheme.py

        except Exception as e:
-            raise Exception(f"Error opening scheme: {e}")
+            raise OSError(f"Error opening scheme: {e}")


@s-weigand Are all this changes in the exceptions done by you or py black?

The Exception changes are all done by me (only in this case pyupgrade changed my IOError to an OSError), so sonar cloud complains about less code smells see s-weigand#14 and #409 .

For the OSError see the changes in python 3.3 https://docs.python.org/3/library/exceptions.html#OSError.filename2

sonarcloud · 2020-09-04T12:12:45Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities (and 1 Security Hotspot to review)
25 Code Smells

No Coverage information
11.8% Duplication

s-weigand added 24 commits August 31, 2020 13:13

Added basic pre-commit config and added pre-commit to requirements_dev

066a1a1

Applied pre-commit-hooks changes to the files

dc3e324

Added .git-blame-ignore-revs so reformatting won't clutter git blame

4abc0b8

See https://git-scm.com/docs/git-blame#Documentation/git-blame.txt---ignore-revltrevgt

Added instructions for pre-commit to contributing guide

b0134ea

Added black to pre-commit and added pyproject.toml for black config

b34d8ec

IGNORE Reformatted code base with black

cd4fd53

Added black reformat commit to .git-blame-ignore-revs

c84057b

Added flake8 to pre-commit

d3ab0ab

Added ignore E231 to flake8 config, due to false positive error

dd55d2e

This is a temporary fix until the issue is resolved, see PyCQA/pycodestyle#914

Replaced flake8 tests with full pre-commit tests suite

cc9c598

Added isort to pre-commit

e664890

IGNORE Reformatted code base with isort

d19e97d

Added isort reformat commit to .git-blame-ignore-revs

d200346

Added checkers for *.rst files

35cf6bb

Fixed rstcheck issues

88c46dd

CONTRIBUTING.rst:60: (INFO/1) Hyperlink target "get-started" is not referenced. docs/source/introduction.rst:9: (WARNING/2) Inline emphasis start-string without end-string.

Added codespell to pre-commit

0ef09e0

Added pyupgrade to pre-commit

eab376b

Applied pyupgrade on code base

5ef5e9c

Added yesqa to pre-commit

425bf47

Removed unnecessary noqa comments and specified errors on general ones

031132d

Fixed version pattern for bumpversion

ea9305f

This was needed since black replaced the single quotes with double quotes

s-weigand added the Type: Tooling Tools used for the project (CI, CD, docs etc.) label Sep 1, 2020

s-weigand requested a review from jsnel September 1, 2020 15:01

s-weigand mentioned this pull request Sep 2, 2020

Fix sonar cloud issues in #407 #408

Closed

s-weigand added 2 commits September 3, 2020 00:25

Set glotaran as known_first_party,

a5fdde2

so isort will import it after the third party imports

IGNORE Reformatted code base with isort

f155c79

jsnel approved these changes Sep 2, 2020

View reviewed changes

s-weigand added 6 commits September 3, 2020 00:26

Added new isort reformat commit to .git-blame-ignore-revs

be4bd35

Fixed pyglotaran being referred to as glotaran in quickstart

b6680a2

Made isort force_single_line, so each import is on a line on its own

c74e25f

IGNORE Reformatted code base with isort

4eaf1ae

forcing import to be each on its own line

Added force_single_line isort reformat commit to .git-blame-ignore-revs

078e24f

s-weigand added 3 commits September 3, 2020 19:56

Updated repo revisions in .pre-commit-config.yaml

9680edf

This can be done automatically by using 'pre-commit autoupdate'

IGNORE Reformatted code base with newer revision of black

09ceed4

Added new black reformat commit to .git-blame-ignore-revs

16c090c

joernweissenborn reviewed Sep 4, 2020

View reviewed changes

Merge branch 'master' into use-pre-commit

92462b5

joernweissenborn merged commit 328357b into glotaran:master Sep 4, 2020

s-weigand deleted the use-pre-commit branch September 8, 2020 08:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use pre commit #407

Use pre commit #407

s-weigand commented Sep 1, 2020

codecov bot commented Sep 1, 2020 •

edited

Loading

s-weigand commented Sep 1, 2020

jsnel commented Sep 1, 2020

jsnel left a comment

jsnel Sep 2, 2020

s-weigand Sep 2, 2020

jsnel Sep 2, 2020

s-weigand commented Sep 2, 2020 •

edited

Loading

joernweissenborn Sep 4, 2020

s-weigand Sep 4, 2020

joernweissenborn Sep 4, 2020

s-weigand Sep 4, 2020

s-weigand Sep 4, 2020

sonarcloud bot commented Sep 4, 2020


		__version__ = '0.1.0'
		__version__ = "0.1.0"

Use pre commit #407

Use pre commit #407

Conversation

s-weigand commented Sep 1, 2020

codecov bot commented Sep 1, 2020 • edited Loading

Codecov Report

s-weigand commented Sep 1, 2020

jsnel commented Sep 1, 2020

jsnel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

s-weigand commented Sep 2, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Sep 4, 2020

codecov bot commented Sep 1, 2020 •

edited

Loading

s-weigand commented Sep 2, 2020 •

edited

Loading