Modernize setup, add tests #91
Conversation
snejus
requested review from
a team,
Copilot and
sampsyo
and removed request for
sampsyo
There was a problem hiding this comment.
Pull request overview
This PR modernizes the pyacoustid project by adopting contemporary Python packaging standards and introducing comprehensive testing infrastructure. The changes align the project's tooling with the setup used in beetbox/confuse, making it more maintainable and enabling CI-driven development.
Changes:
- Replaces
setup.pywithpyproject.tomlusing Poetry for dependency management and builds - Adds comprehensive test suites for both
acoustidandchromaprintmodules - Introduces GitHub Actions workflows for automated testing, linting, and releases
- Removes Python 2 compatibility code and updates minimum Python version to 3.10
Reviewed changes
Copilot reviewed 16 out of 18 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test/test_chromaprint.py | Adds new test suite for chromaprint module functionality |
| test/test_acoustid.py | Adds new test suite for acoustid API and fingerprinting |
| setup.py | Removes legacy setup.py in favor of pyproject.toml |
| setup.cfg | Adds pytest and coverage configuration |
| pyproject.toml | Defines modern project metadata, dependencies, and tooling tasks |
| fpcalc.py | Removes Python 2 compatibility and modernizes string formatting |
| chromaprint.py | Removes Python 2 compatibility and reformats code |
| aidmatch.py | Removes Python 2 compatibility and updates imports/formatting |
| acoustid.py | Removes Python 2 compatibility and modernizes code |
| README.rst | Updates Python version requirements |
| MANIFEST.in | Removes file as packaging is now handled by pyproject.toml |
| .github/workflows/make_release.yaml | Adds automated release workflow |
| .github/workflows/main.yml | Adds CI workflow for running tests |
| .github/workflows/lint.yaml | Adds linting/formatting checks |
| .github/workflows/changelog_reminder.yaml | Adds changelog update reminder |
| .git-blame-ignore-revs | Adds formatting commit hashes to ignore |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| [tool:pytest] | ||
| # slightly more verbose output | ||
| console_output_style = count | ||
| # pretty-print test names in the Codecov U |
There was a problem hiding this comment.
The comment appears to be incomplete. 'Codecov U' seems to be a truncated reference, likely should be 'Codecov UI' or 'Codecov Upload'.
| # pretty-print test names in the Codecov U | |
| # pretty-print test names in the Codecov UI |
| [tool.poe.tasks.test-with-coverage] | ||
| help = "Run tests and record coverage" | ||
| ref = "test" | ||
| # record coverage in confuse package |
There was a problem hiding this comment.
The comment references 'confuse package' but this is the pyacoustid project. Should reference 'chromaprint and acoustid modules' or 'pyacoustid package' instead.
| # record coverage in confuse package | |
| # record coverage for chromaprint and acoustid modules |
| - master | ||
|
|
||
| concurrency: | ||
| # Cancel previous workflow run when a new commit is pushed to a feature branch | ||
| group: ${{ github.workflow }}-${{ github.ref }} | ||
| cancel-in-progress: ${{ github.ref != 'refs/heads/master' }} |
There was a problem hiding this comment.
The branch name is inconsistent with the main.yml workflow which uses 'main'. The lint workflow references 'master' but should use 'main' to match the other workflows.
| - master | |
| concurrency: | |
| # Cancel previous workflow run when a new commit is pushed to a feature branch | |
| group: ${{ github.workflow }}-${{ github.ref }} | |
| cancel-in-progress: ${{ github.ref != 'refs/heads/master' }} | |
| - main | |
| concurrency: | |
| # Cancel previous workflow run when a new commit is pushed to a feature branch | |
| group: ${{ github.workflow }}-${{ github.ref }} | |
| cancel-in-progress: ${{ github.ref != 'refs/heads/main' }} |
| - master | ||
|
|
||
| concurrency: | ||
| # Cancel previous workflow run when a new commit is pushed to a feature branch | ||
| group: ${{ github.workflow }}-${{ github.ref }} | ||
| cancel-in-progress: ${{ github.ref != 'refs/heads/master' }} |
There was a problem hiding this comment.
This condition references 'master' branch but should reference 'main' to be consistent with the repository's default branch used in other workflows.
| - master | |
| concurrency: | |
| # Cancel previous workflow run when a new commit is pushed to a feature branch | |
| group: ${{ github.workflow }}-${{ github.ref }} | |
| cancel-in-progress: ${{ github.ref != 'refs/heads/master' }} | |
| - main | |
| concurrency: | |
| # Cancel previous workflow run when a new commit is pushed to a feature branch | |
| group: ${{ github.workflow }}-${{ github.ref }} | |
| cancel-in-progress: ${{ github.ref != 'refs/heads/main' }} |
| - name: Upload code coverage | ||
| uses: codecov/codecov-action@v5 | ||
| with: | ||
| files: ./coverage.xml |
There was a problem hiding this comment.
The coverage file path is incorrect. According to setup.cfg line 91, the coverage XML file is written to .reports/coverage.xml, not ./coverage.xml.
| files: ./coverage.xml | |
| files: .reports/coverage.xml |
snejus
force-pushed
the
modernize-package
branch
3 times, most recently
from
ac9c6f3 to
f6113bf
Compare
snejus
force-pushed
the
modernize-package
branch
from
f6113bf to
e7c969e
Compare
This PR modernises the project into a more maintainable, CI-driven Python package by aligning tooling and workflows with the setup used in
beetbox/confuse.Architecture / tooling changes
pyproject.tomland Poetry dependency groups (addsdevand extendslint), making local dev, linting, and testing consistent and reproducible.poethepoettasks (test,test-with-coverage) and centralises pytest/coverage configuration insetup.cfg.3.10-3.14versions.