Initial Linting Cleanup

[genedan]

Ruff has now been added as a workflow with the following rules: E2, E4, E7, E9, and F. According to these rules, the project has a large number of existing errors that need to be cleaned up. Because of this, the workflow has been configured to only run on changed files in a pull request. Otherwise, PRs will be bogged down with linting errors on unrelated files.

The exclusion of existing errors has been documented in pyproject.toml under the header [tool.ruff.lint.per-file-ignores]:

chainladder-python/pyproject.toml

Lines 111 to 156 in 52be42a

	[tool.ruff.lint.per-file-ignores]
	"chainladder/adjustments/bootstrap.py" = ["E721", "E722", "F841"]
	"chainladder/adjustments/disposal.py" = ["F401"]
	"chainladder/adjustments/tests/test_berqsherm.py" = ["F841"]
	"chainladder/adjustments/tests/test_disposal.py" = ["F841"]
	"chainladder/adjustments/trend.py" = ["F401"]
	"chainladder/core/base.py" = ["E721"]
	"chainladder/core/common.py" = ["F401"]
	"chainladder/core/correlation.py" = ["E741"]
	"chainladder/core/dunders.py" = ["E721", "E722", "F841"]
	"chainladder/core/pandas.py" = ["E721", "F841"]
	"chainladder/core/slice.py" = ["E712", "E721", "E741"]
	"chainladder/core/tests/rtest_correlation.py" = ["E722", "F821"]
	"chainladder/core/tests/test_arithmetic.py" = ["E711"]
	"chainladder/core/tests/test_display.py" = ["E722"]
	"chainladder/core/tests/test_grain.py" = ["F401", "F841"]
	"chainladder/core/tests/test_triangle.py" = ["E712", "E721", "F811", "F841"]
	"chainladder/core/triangle.py" = ["E712", "E721", "E731", "F401", "F841"]
	"chainladder/development/base.py" = ["E712", "F401", "F841"]
	"chainladder/development/clark.py" = ["E721", "E731"]
	"chainladder/development/constant.py" = ["E712"]
	"chainladder/development/glm.py" = ["F401"]
	"chainladder/development/incremental.py" = ["E721", "F401"]
	"chainladder/development/learning.py" = ["E711", "F401"]
	"chainladder/development/tests/rtest_clark.py" = ["E722"]
	"chainladder/development/tests/rtest_development.py" = ["E722", "F401"]
	"chainladder/development/tests/rtest_munich.py" = ["E722", "F401"]
	"chainladder/development/tests/test_constant.py" = ["F841"]
	"chainladder/development/tests/test_development.py" = ["E722"]
	"chainladder/development/tests/test_incremental.py" = ["F841"]
	"chainladder/development/tests/test_munich.py" = ["F841"]
	"chainladder/methods/base.py" = ["F401"]
	"chainladder/methods/capecod.py" = ["F401"]
	"chainladder/methods/mack.py" = ["F401"]
	"chainladder/methods/tests/rtest_mack.py" = ["E722", "F401"]
	"chainladder/tails/bondy.py" = ["F401"]
	"chainladder/tails/curve.py" = ["E721", "F401"]
	"chainladder/tails/tests/rtest_exponential.py" = ["E722", "F401"]
	"chainladder/tails/tests/test_constant.py" = ["F401"]
	"chainladder/utils/cupy.py" = ["E722", "F401"]
	"chainladder/utils/dask.py" = ["E722", "E741"]
	"chainladder/utils/sparse.py" = ["E721"]
	"chainladder/utils/tests/test_utilities.py" = ["E712", "E741", "F811", "F841"]
	"chainladder/utils/triangle_weight.py" = ["F401"]
	"chainladder/utils/utility_functions.py" = ["E721", "F401"]
	"chainladder/workflow/tests/test_voting.py" = ["E731"]

These exclusions only apply if a file remains untouched in a pull request. Once someone edits the file, all rules are checked against the entire file, so the author of the PR will be responsible for the whole file's cleanup at that time.

Once the file has been cleaned up, the pull request author should remove its entry from [tool.ruff.lint.per-file-ignores]. We will proceed in this manner until [tool.ruff.lint.per-file-ignores] no longer has any entries.

Once the cleanup is complete, we will update the workflow to check the entire project on each pull request.

Adding/Removing Rules

Ruff has a lot of rules. Because of this, I believe it is impossible to know all of the rules we need to apply ahead of time, but I do expect us to add rules over time as we discover errors that weren't caught by the initial ruleset, which was recommended to me by Claude. For example, E2 was not part of the first attempt. Once we found out that whitespace around operators was not enforced, we added it, which led to the discovery of even more errors in the project base, causing the list of entries in [tool.ruff.lint.per-file-ignores] to expand.

Likewise, we may find certain rules to be too strict and remove them. It will be a judgment call as to how broad we apply new rules - too broad, and we may find the linter to be overly strict, but too narrow, and we might not catch enough errors.

When I added E2, that covers all the rules in the E200s, for example, E251, E252, are both covered by simply specifying "E2". I believe that we need almost all the rules in E2, so I added it broadly. But if I am wrong on this, we need to be more granular on the rules.

Voting

We may need to apply some rules that are a matter of style preference. I believe most of these can be resolved by simple discussion between the contributors, but if any of them are contentious, we can resolve them by simple majority voting.

Resolution Criteria

This issue can be marked as closed when:

• [tool.ruff.lint.per-file-ignores] no longer has entries (to the fullest extent possible)
• The Ruff workflow is updated to check the entire project, rather than the PR patch
• Pre-commit checks are likewise updated to check the entire project.