Skip to content

Commit

Permalink
Migrate from isort/Black/Flake8 to Ruff for Code Formatting and Linti…
Browse files Browse the repository at this point in the history
…ng (#5424)

#### Context
I noticed one of recently merged PRs used `unittest` assertions that we
are migrating from. Thus, I thought it would be a good idea to configure
our linting to catch such issues in the future. Since I was here, I
replaced our multiple linting and formatting tools with a single tool,
Ruff, to simplify and speed up our development workflow.

#### Summary
This PR migrates the codebase from using isort, Black and Flake8 to Ruff
for code formatting and linting. The changes include updates to the
GitHub Actions workflow, pre-commit configuration, and documentation to
reflect the new tooling.

#### Changes
1. **GitHub Actions Workflow**
- Updated `.github/workflows/lint.yml` to use `poetry install
--only=lint` and `poe lint --output-format=github`.

2. **Pre-commit Configuration**
   - Replaced Black and Isort with Ruff in `.pre-commit-config.yaml`.

3. **Documentation**
- Updated `CONTRIBUTING.rst` to reflect the use of Ruff for formatting
and linting.
- Modified instructions for running tests and handling external API
requests.

4. **Poetry Configuration**
- Removed Black, Isort, Flake8, and related dependencies from
`poetry.lock` and `pyproject.toml`.
   - Added Ruff as the new linter and formatter in `pyproject.toml`.

5. **Setup Configuration**
   - Removed Flake8 configuration from `setup.cfg`.

6. **Git blame**
- Introduced `.git-blame-ignore-revs` file to keep git blame clean from
formatting
changes. Configure your local `beets` repository to use this file by
running:
   ```fish
   $ git config --local blame.ignoreRevsFile .git-blame-ignore-revs
   ```

#### Benefits
- **Performance**: Ruff is known for its speed and efficiency, which
should improve the developer experience.
- **Consolidation**: Using a single tool for both formatting and linting
simplifies the development workflow.

#### How to Test
1. **Linting and Formatting**
   - Run `poe check-format` to check for formatting issues.
   - Run `poe format` to format the codebase.
   - Run `poe lint` to check for linting issues.

2. **Pre-commit Hooks**
- Ensure pre-commit hooks are working correctly by running `pre-commit
run --all-files`.

3. **CI Pipeline**
   - Verify that the GitHub Actions workflow completes successfully.

#### Notes
- Contributions migrating existing tests from `unittest` to `pytest` are
welcome.
- External API requests should be mocked using `requests_mock` and
tested weekly in the integration test suite.

#### References
- [Ruff Documentation](https://docs.astral.sh/ruff/)
- [Pre-commit Hooks](https://pre-commit.com/hooks.html)

---

This PR aims to streamline our development process by adopting Ruff, a
modern and efficient tool for Python code formatting and linting.
  • Loading branch information
snejus authored Sep 21, 2024
2 parents 9f29c36 + 11fa6c7 commit 88d3f04
Show file tree
Hide file tree
Showing 121 changed files with 555 additions and 795 deletions.
45 changes: 45 additions & 0 deletions .git-blame-ignore-revs
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# 2014
# flake8-cleanliness in missing
e21c04e9125a28ae0452374acf03d93315eb4381

# 2016
# Removed unicode_literals from library, logging and mediafile
43572f50b0eb3522239d94149d91223e67d9a009
# Removed unicode_literals from plugins
53d2c8d9db87be4d4750ad879bf46176537be73f
# reformat flake8 errors
1db46dfeb6607c164afb247d8da82443677795c1

# 2021
# pyupgrade root
e26276658052947e9464d9726b703335304c7c13
# pyupgrade beets dir
6d1316f463cb7c9390f85bf35b220e250a35004a
# pyupgrade autotag dir
f8b8938fd8bbe91898d0982552bc75d35703d3ef
# pyupgrade dbcore dir
d288f872903c79a7ee7c5a7c9cc690809441196e
# pyupgrade ui directory
432fa557258d9ff01e23ed750f9a86a96239599e
# pyupgrade util dir
af102c3e2f1c7a49e99839e2825906fe01780eec
# fix unused import and flake8
910354a6c617ed5aa643cff666205b43e1557373
# pyupgrade beetsplug and tests
1ec87a3bdd737abe46c6e614051bf9e314db4619

# 2022
# Reformat flake8 config comments
abc3dfbf429b179fac25bd1dff72d577cd4d04c7

# 2023
# Apply formatting tools to all files
a6e5201ff3fad4c69bf24d17bace2ef744b9f51b

# 2024
# Reformat the codebase
85a17ee5039628a6f3cdcb7a03d7d1bd530fbe89
# Fix lint issues
f36bc497c8c8f89004f3f6879908d3f0b25123e1
# Remove some lint exclusions and fix the issues
5f78d1b82b2292d5ce0c99623ba0ec444b80d24c
7 changes: 2 additions & 5 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ jobs:
cache: poetry

- name: Install dependencies
run: poetry install --only=format
run: poetry install --only=lint

- name: Check code formatting
# the job output will contain colored diffs with what needs adjusting
Expand All @@ -84,10 +84,7 @@ jobs:
run: poetry install --only=lint

- name: Lint code
uses: liskin/gh-problem-matcher-wrap@v3
with:
linters: flake8
run: poe lint ${{ needs.changed-files.outputs.changed_python_files }}
run: poe lint --output-format=github ${{ needs.changed-files.outputs.changed_python_files }}

mypy:
if: needs.changed-files.outputs.any_python_changed == 'true'
Expand Down
12 changes: 3 additions & 9 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,7 @@
# See https://pre-commit.com/hooks.html for more hooks

repos:
- repo: https://github.com/psf/black
rev: 24.2.0
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.6.6
hooks:
- id: black

- repo: https://github.com/pycqa/isort
rev: 5.13.2
hooks:
- id: isort
name: isort (python)
- id: ruff-format
45 changes: 20 additions & 25 deletions CONTRIBUTING.rst
Original file line number Diff line number Diff line change
Expand Up @@ -274,14 +274,13 @@ There are a few coding conventions we use in beets:
Style
-----

We follow `black`_ formatting and `google's docstring format`_.
We use `ruff`_ to format and lint the codebase.

Use ``poe check-format`` and ``poe lint`` to check your code for style and
Run ``poe check-format`` and ``poe lint`` to check your code for style and
linting errors. Running ``poe format`` will automatically format your code
according to the specifications required by the project.

.. _black: https://black.readthedocs.io/en/stable/
.. _google's docstring format: https://google.github.io/styleguide/pyguide.html#38-comments-and-docstrings
.. _ruff: https://docs.astral.sh/ruff/

Handling Paths
--------------
Expand Down Expand Up @@ -345,10 +344,10 @@ environment variable ``SKIP_SLOW_TESTS``, for example::
Coverage
^^^^^^^^

Coverage is measured automatically when running the tests. If you find it takes
a while to calculate, disable it::
The ``test`` command does not include coverage as it slows down testing. In
order to measure it, use the ``test-with-coverage`` task

$ poe test --no-cov
$ poe test-with-coverage [pytest options]

You are welcome to explore coverage by opening the HTML report in
``.reports/html/index.html``.
Expand Down Expand Up @@ -379,28 +378,24 @@ Writing Tests
Writing tests is done by adding or modifying files in folder `test`_.
Take a look at
`https://github.com/beetbox/beets/blob/master/test/test_template.py#L224`_
to get a basic view on how tests are written. We currently allow writing
tests with either `unittest`_ or `pytest`_.
to get a basic view on how tests are written. Since we are currently migrating
the tests from `unittest`_ to `pytest`_, new tests should be written using
`pytest`_. Contributions migrating existing tests are welcome!

Any tests that involve sending out network traffic e.g. an external API
call, should be skipped normally and run under our weekly `integration
test`_ suite. These tests can be useful in detecting external changes
that would affect ``beets``. In order to do this, simply add the
following snippet before the applicable test case:
External API requests under test should be mocked with `requests_mock`_,
However, we still want to know whether external APIs are up and that they
return expected responses, therefore we test them weekly with our `integration
test`_ suite.

.. code-block:: python
In order to add such a test, mark your test with the ``integration_test`` marker

@unittest.skipUnless(
os.environ.get('INTEGRATION_TEST', '0') == '1',
'integration testing not enabled')
.. code-block:: python
If you do this, it is also advised to create a similar test that 'mocks'
the network call and can be run under normal circumstances by our CI and
others. See `unittest.mock`_ for more info.
@pytest.mark.integration_test
def test_external_api_call():
...
- **AVOID** using the ``start()`` and ``stop()`` methods of
``mock.patch``, as they require manual cleanup. Use the annotation or
context manager forms instead.
This way, the test will be run only in the integration test suite.

.. _Codecov: https://codecov.io/github/beetbox/beets
.. _pytest-random: https://github.com/klrmn/pytest-random
Expand All @@ -410,6 +405,6 @@ others. See `unittest.mock`_ for more info.
.. _`https://github.com/beetbox/beets/blob/master/test/test_template.py#L224`: https://github.com/beetbox/beets/blob/master/test/test_template.py#L224
.. _unittest: https://docs.python.org/3/library/unittest.html
.. _integration test: https://github.com/beetbox/beets/actions?query=workflow%3A%22integration+tests%22
.. _unittest.mock: https://docs.python.org/3/library/unittest.mock.html
.. _requests-mock: https://requests-mock.readthedocs.io/en/latest/response.html
.. _documentation: https://beets.readthedocs.io/en/stable/
.. _vim: https://www.vim.org/
1 change: 0 additions & 1 deletion beets/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
`python -m beets`.
"""


import sys

from .ui import main
Expand Down
1 change: 0 additions & 1 deletion beets/art.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
music and items' embedded album art.
"""


import os
from tempfile import NamedTemporaryFile

Expand Down
35 changes: 25 additions & 10 deletions beets/autotag/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,23 +12,38 @@
# The above copyright notice and this permission notice shall be
# included in all copies or substantial portions of the Software.

"""Facilities for automatically determining files' correct metadata.
"""
"""Facilities for automatically determining files' correct metadata."""

from typing import Mapping, Sequence, Union

from beets import config, logging
from beets.library import Album, Item

# Parts of external interface.
from .hooks import ( # noqa
AlbumInfo,
AlbumMatch,
Distance,
TrackInfo,
TrackMatch,
from .hooks import AlbumInfo, AlbumMatch, Distance, TrackInfo, TrackMatch
from .match import (
Proposal,
Recommendation,
current_metadata,
tag_album,
tag_item,
)
from .match import Recommendation # noqa
from .match import Proposal, current_metadata, tag_album, tag_item # noqa

__all__ = [
"AlbumInfo",
"AlbumMatch",
"Distance",
"TrackInfo",
"TrackMatch",
"Proposal",
"Recommendation",
"apply_album_metadata",
"apply_item_metadata",
"apply_metadata",
"current_metadata",
"tag_album",
"tag_item",
]

# Global logger.
log = logging.getLogger("beets")
Expand Down
2 changes: 1 addition & 1 deletion beets/autotag/hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ def __init__(self):
self.tracks: Dict[TrackInfo, Distance] = {}

@cached_classproperty
def _weights(cls) -> Dict[str, float]: # noqa: N805
def _weights(cls) -> Dict[str, float]:
"""A dictionary from keys to floating-point weights."""
weights_view = config["match"]["distance_weights"]
weights = {}
Expand Down
5 changes: 2 additions & 3 deletions beets/autotag/match.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@
releases and tracks.
"""


from __future__ import annotations

import datetime
import re
from enum import IntEnum
from typing import (
Any,
Dict,
Expand Down Expand Up @@ -48,7 +48,6 @@
)
from beets.library import Item
from beets.util import plurality
from beets.util.enumeration import OrderedEnum

# Artist signals that indicate "various artists". These are used at the
# album level to determine whether a given release is likely a VA
Expand All @@ -63,7 +62,7 @@
# Recommendation enumeration.


class Recommendation(OrderedEnum):
class Recommendation(IntEnum):
"""Indicates a qualitative suggestion to the user about what should
be done with a given match.
"""
Expand Down
6 changes: 3 additions & 3 deletions beets/autotag/mb.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
# The above copyright notice and this permission notice shall be
# included in all copies or substantial portions of the Software.

"""Searches for albums in the MusicBrainz database.
"""
"""Searches for albums in the MusicBrainz database."""

from __future__ import annotations

import re
Expand Down Expand Up @@ -54,7 +54,7 @@
musicbrainzngs.set_useragent("beets", beets.__version__, "https://beets.io/")


class MusicBrainzAPIError(util.HumanReadableException):
class MusicBrainzAPIError(util.HumanReadableError):
"""An error while talking to MusicBrainz. The `query` field is the
parameter to the action and may have any type.
"""
Expand Down
16 changes: 15 additions & 1 deletion beets/dbcore/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,18 @@
)
from .types import Type

# flake8: noqa
__all__ = [
"AndQuery",
"Database",
"FieldQuery",
"InvalidQueryError",
"MatchQuery",
"Model",
"OrQuery",
"Query",
"Results",
"Type",
"parse_sorted_query",
"query_from_strings",
"sort_from_strings",
]
4 changes: 1 addition & 3 deletions beets/dbcore/db.py
Original file line number Diff line number Diff line change
Expand Up @@ -1224,9 +1224,7 @@ def _make_attribute_table(self, flex_table: str):
UNIQUE(entity_id, key) ON CONFLICT REPLACE);
CREATE INDEX IF NOT EXISTS {0}_by_entity
ON {0} (entity_id);
""".format(
flex_table
)
""".format(flex_table)
)

# Querying.
Expand Down
4 changes: 2 additions & 2 deletions beets/dbcore/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
# The above copyright notice and this permission notice shall be
# included in all copies or substantial portions of the Software.

"""Representation of type information for DBCore model fields.
"""
"""Representation of type information for DBCore model fields."""

import typing
from abc import ABC
from typing import Any, Generic, List, TypeVar, Union, cast
Expand Down
9 changes: 4 additions & 5 deletions beets/importer.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@
log = logging.getLogger("beets")


class ImportAbort(Exception):
class ImportAbortError(Exception):
"""Raised when the user aborts the tagging operation."""

pass
Expand Down Expand Up @@ -360,7 +360,7 @@ def run(self):
pl.run_parallel(QUEUE_SIZE)
else:
pl.run_sequential()
except ImportAbort:
except ImportAbortError:
# User aborted operation. Silently stop.
pass

Expand Down Expand Up @@ -627,8 +627,7 @@ def finalize(self, session):
self.save_progress()
if session.config["incremental"] and not (
# Should we skip recording to incremental list?
self.skip
and session.config["incremental_skip_later"]
self.skip and session.config["incremental_skip_later"]
):
self.save_history()

Expand Down Expand Up @@ -947,7 +946,7 @@ def remove_replaced(self, lib):
dup_item.remove()
log.debug(
"{0} of {1} items replaced",
sum(bool(l) for l in self.replaced_items.values()),
sum(bool(v) for v in self.replaced_items.values()),
len(self.imported_items()),
)

Expand Down
Loading

0 comments on commit 88d3f04

Please sign in to comment.