Update pre-commits and pytest configuration

lang-m commented 7 months ago

User description

pre-commit: use ruff and ruff formatting to replace black, isort, flake8
pre-commit: remove hard-coded Python version
tests: convert all warnings to errors

Type

enhancement, bug_fix

Description

Updated pre-commit hooks and configurations to use newer versions and streamline settings.
Removed flake8 configurations from setup.cfg as the project transitions to using ruff for linting.
Enhanced Python project settings in pyproject.toml to handle specific false positives and improve test configurations.
Minor code and formatting improvements across various Python modules for better readability and maintenance.

Changes walkthrough

Relevant files

Configuration changes

3 files

.pre-commit-config.yaml `Update and streamline pre-commit configurations` .pre-commit-config.yaml Updated versions for pre-commit hooks. Removed unused configurations. Added new hook configurations.	+2/-5
pyproject.toml `Enhance Python project configurations` pyproject.toml Added new configurations for `tool.coverage.run` and `tool.pytest.ini_options`. Updated `tool.ruff` settings to handle notebook specific false positives.	+14/-7
setup.cfg `Remove flake8 configurations` setup.cfg - Removed all flake8 configurations.	+0/-28

Enhancement

1 files

field.py

Simplify dictionary comprehension in field.py

discretisedfield/field.py - Simplified dictionary comprehension in `_r_dim_mapping` property.

+1/-1

Formatting

10 files

__init__.py `Minor formatting improvement in __init__.py` discretisedfield/__init__.py - Added a blank line for better code organization.	+1/-0
__init__.py `Minor formatting improvement in io/__init__.py` discretisedfield/io/__init__.py - Added a blank line for clarity.	+1/-0
ovf2vtk.py `Minor formatting improvement in ovf2vtk.py` discretisedfield/io/ovf2vtk.py - Added a blank line for clarity.	+1/-0
__init__.py `Minor formatting improvement in plotting/__init__.py` discretisedfield/plotting/__init__.py - Added a blank line for clarity.	+1/-0
hv.py `Minor formatting improvement in hv.py` discretisedfield/plotting/hv.py - Added a blank line for clarity.	+1/-0
k3d_field.py `Minor formatting improvement in k3d_field.py` discretisedfield/plotting/k3d_field.py - Added a blank line for clarity.	+1/-0
mpl.py `Minor formatting improvement in mpl.py` discretisedfield/plotting/mpl.py - Added a blank line for clarity.	+1/-0
mpl_field.py `Minor formatting improvement in mpl_field.py` discretisedfield/plotting/mpl_field.py - Added a blank line for clarity.	+1/-0
__init__.py `Minor formatting improvement in tools/__init__.py` discretisedfield/tools/__init__.py - Added a blank line for clarity.	+1/-0
tasks.py `Minor formatting improvement in tasks.py` tasks.py - Added a blank line for clarity.	+1/-0

✨ PR-Agent usage: Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

github-actions[bot] commented 7 months ago

PR Description updated to latest commit (https://github.com/ubermag/discretisedfield/commit/20b7dff831875f42077b3520eb9cebbaf38ba085)

github-actions[bot] commented 7 months ago

PR Review

⏱️ Estimated effort to review [1-5]	3, because the PR includes a variety of changes across multiple configuration files and Python scripts. The changes involve updates to pre-commit hooks, removal of old linting configurations, and adjustments in testing configurations. Understanding the implications of these changes, especially in the context of continuous integration and code quality, requires a thorough review.
🧪 Relevant tests	No
🔍 Possible issues	Possible Bug: The change in `discretisedfield/field.py` modifies the behavior of the `_r_dim_mapping` method by removing the default return value of `None` for missing keys in the dictionary comprehension. This could lead to a `KeyError` if `reversed_mapping` does not contain a key found in `self.mesh.region.dims`.
🔒 Security concerns	No

Code feedback:

relevant file	discretisedfield/field.py
suggestion	Consider restoring the default value of `None` in the dictionary comprehension to avoid potential `KeyError`. This can be done by modifying the line to include a default return value when the key is not found in the `reversed_mapping`. [important]
relevant line	return {dim: reversed_mapping.get(dim) for dim in self.mesh.region.dims}

✨ Review tool usage guide:

**Overview:** The `review` tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be [added](https://pr-agent-docs.codium.ai/tools/review/#general-configurations) by configuring the tool. The tool can be triggered [automatically](https://pr-agent-docs.codium.ai/usage-guide/automations_and_usage/#github-app-automatic-tools-when-a-new-pr-is-opened) every time a new PR is opened, or can be invoked manually by commenting on any PR. - When commenting, to edit [configurations](https://github.com/Codium-ai/pr-agent/blob/main/pr_agent/settings/configuration.toml#L23) related to the review tool (`pr_reviewer` section), use the following template: ``` /review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=... ``` - With a [configuration file](https://pr-agent-docs.codium.ai/usage-guide/configuration_options/), use the following template: ``` [pr_reviewer] some_config1=... some_config2=... ``` See the review [usage page](https://pr-agent-docs.codium.ai/tools/review/) for a comprehensive guide on using this tool.

github-actions[bot] commented 7 months ago

PR Code Suggestions

Category	Suggestions
Bug	Add a default value in dictionary lookup to prevent potential `None` values. ___ In the `_r_dim_mapping` method, the `reversed_mapping.get(dim)` call should include a default value to handle cases where `dim` is not present in `reversed_mapping`. This prevents the method from returning `None` for some dimensions, which could lead to unexpected behavior or errors in other parts of the code that rely on this mapping. [discretisedfield/field.py [604]](https://github.com/ubermag/discretisedfield/pull/525/files#diff-c58c98f0ea2639669e6b8e97bd7e9eedc3cb071080df2e78678b2e70fe76cdd4R604-R604) ```diff -return {dim: reversed_mapping.get(dim) for dim in self.mesh.region.dims} +return {dim: reversed_mapping.get(dim, dim) for dim in self.mesh.region.dims} ```
Enhancement	Update the revision number in pre-commit configuration to the latest version. ___ Update the `rev` version for the `pre-commit/pre-commit-hooks` to `v4.5.0` to match the new version specified in the PR. This ensures that all hooks use the latest versions for consistency and to leverage any new features or bug fixes in the hooks. [.pre-commit-config.yaml [5]](https://github.com/ubermag/discretisedfield/pull/525/files#diff-63a9c44a44acf85fea213a857769990937107cf072831e1a26808cfde9d096b9R5-R5) ```diff -rev: v4.4.0 +rev: v4.5.0 ```
Enhancement	Expand the list of ignored warnings in pytest configuration for clearer test outputs. ___ It's beneficial to specify a more comprehensive list of warnings to ignore in the pytest configuration to avoid common false positives and focus on significant warnings during testing. [pyproject.toml [112-115]](https://github.com/ubermag/discretisedfield/pull/525/files#diff-50c86b7ed8ac2cf95bd48334961bf0530cdc77b5a56f852c5c61b89d735fd711R112-R115) ```diff filterwarnings = [ + "error", + "ignore:DeprecationWarning", + "ignore:UserWarning", + "ignore:((.\|\n))Sentinel is not a public part of the traitlets API((.\|\n))" +] ```
Best practice	Refine the pattern for omitting files in coverage configuration to improve accuracy. ___ The `omit` configuration in `[tool.coverage.run]` should include more specific patterns to ensure that only test files are omitted from coverage reports, rather than excluding all files under the `tests` directory. This change helps in maintaining accurate test coverage metrics. [pyproject.toml [73]](https://github.com/ubermag/discretisedfield/pull/525/files#diff-50c86b7ed8ac2cf95bd48334961bf0530cdc77b5a56f852c5c61b89d735fd711R73-R73) ```diff -omit = ["discretisedfield/tests/"] +omit = ["discretisedfield/tests//.py"] ```
Best practice	Reintroduce the trailing whitespace hook for cleaner code. ___ Consider reintroducing the `trailing-whitespace` hook in the pre-commit configuration. This hook is useful for maintaining clean code by automatically removing unnecessary trailing whitespaces, which are generally considered bad practice in many coding standards. [.pre-commit-config.yaml [7-10]](https://github.com/ubermag/discretisedfield/pull/525/files#diff-63a9c44a44acf85fea213a857769990937107cf072831e1a26808cfde9d096b9R7-R10) ```diff hooks: + - id: trailing-whitespace ```

✨ Improve tool usage guide:

**Overview:** The `improve` tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered [automatically](https://pr-agent-docs.codium.ai/usage-guide/automations_and_usage/#github-app-automatic-tools-when-a-new-pr-is-opened) every time a new PR is opened, or can be invoked manually by commenting on a PR. - When commenting, to edit [configurations](https://github.com/Codium-ai/pr-agent/blob/main/pr_agent/settings/configuration.toml#L78) related to the improve tool (`pr_code_suggestions` section), use the following template: ``` /improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=... ``` - With a [configuration file](https://pr-agent-docs.codium.ai/usage-guide/configuration_options/), use the following template: ``` [pr_code_suggestions] some_config1=... some_config2=... ``` See the improve [usage page](https://pr-agent-docs.codium.ai/tools/improve/) for a comprehensive guide on using this tool.

ubermag / discretisedfield