Modernize packaging to uv_build/ruff/lefthook (numpy-stl template)#122
Conversation
Co-authored-by: Hartmut Goebel <htgoebel@users.noreply.github.com>
Fix issue #109: Remove trailing BIC text from purpose field
Build/packaging: - Switch build backend from setuptools to uv_build; drop the spurious setuptools runtime dependency (no runtime deps remain). - SPDX license metadata, requires-python >=3.10, README.md, cleaned classifiers; single-source the version via importlib.metadata. - Migrate pytest config into pyproject; mypy strict; add pyrefly and coverage path mapping; dev tools in a PEP 735 dependency-group. Tooling/CI: - tox.toml -> tox.ini (py310-313, lint, pyright, mypy, pyrefly, docs, coverage, codespell, repo-review) with tox-gh-actions and a coverage combine step enforcing 100%. - Replace single-job workflow with split ci.yml plus publish.yml (PyPI Trusted Publishing) and codeql.yml; dependabot targets develop. - Replace pre-commit with lefthook. Docs: - Convert README.rst to README.md (absolute URLs); furo theme; bump Read the Docs to ubuntu-24.04 / Python 3.13. Cleanup: - Remove MANIFEST.in, Makefile, pytest.ini, tox.toml, and the duplicate requirements*.txt files; gitignore uv.lock, caches, and scratch files. - Minor type/lint fixes in models.py, tags.py, and test_sta_parsing.py.
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
There was a problem hiding this comment.
Code Review
This pull request modernizes the project's build system and development tooling by migrating to uv and uv_build, updating the minimum Python requirement to 3.10+, and replacing pre-commit with lefthook. It also refactors transaction details processing in mt940/processors.py to clean up trailing identifiers in the purpose and segment fields, with corresponding updates to the test suite. The review feedback suggests leveraging Python 3.10+ features by replacing manual slicing and regular expression substitutions with str.removesuffix() for cleaner and safer string manipulation.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
- processors: use str.removesuffix() instead of slice magic numbers and re.sub() for trailing BIC/IBAN markers (gemini-code-assist). - tests: assertIn() instead of assertTrue(x in y) for clearer failures (CodeQL).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fcfdd15710
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
This PR modernizes the mt940 project’s packaging, tooling, CI, and documentation to a uv/ruff/tox/lefthook-based workflow (aligned with the numpy-stl template), while also including a partial fix related to Issue #109 around transaction-details parsing.
Changes:
- Migrates build/metadata to
pyproject.tomlwithuv_build, updated Python support (>=3.10), and consolidated dev tooling configuration. - Replaces legacy automation (old tox config, pre-commit, single workflow) with
tox.ini, lefthook, and split GitHub Actions workflows (CI/publish/CodeQL). - Updates docs and test fixtures; adds/adjusts parsing-related logic and tests (Issue #109 segment cleanup).
Reviewed changes
Copilot reviewed 89 out of 92 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tox.toml | Removes old tox configuration (replaced by tox.ini). |
| tox.ini | New tox env matrix for tests/lint/type-check/docs/coverage and GH Actions mapping. |
| ruff.toml | Updates Ruff target version to Python 3.10. |
| requirements.txt | Removes legacy runtime requirements file. |
| README.rst | Removes legacy README in reStructuredText. |
| README.md | Adds new Markdown README with updated tooling instructions and links. |
| pytest.ini | Removes legacy pytest.ini (migrated into pyproject.toml). |
| pyproject.toml | Switches build backend to uv_build; centralizes tool configs (pytest/mypy/pyright/coverage/repo-review/etc.). |
| mt940/tags.py | Tightens typing/metadata for Tag classes (adds name attribute). |
| mt940/processors.py | Updates MT940 detail parsing and adds Issue #109 cleanup logic. |
| mt940/models.py | Import ordering/type-related adjustments. |
| mt940/init.py | Exports package __version__. |
| mt940/about.py | Single-sources version via importlib.metadata with fallback. |
| mt940_tests/test_sta_parsing.py | Refactors STA discovery and improves typing; uses zip(..., strict=False). |
| mt940_tests/test_processors_issue109.py | Adds tests for Issue #109-related segment cleanup. |
| mt940_tests/test_issues/test_purpose_field_formatting.py | Adds an Issue #109 regression test (currently placed under a subdir). |
| mt940_tests/test_issues/init.py | Adds package marker for test_issues directory. |
| mt940_tests/self-provided/wrapped_timestamp.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/self-provided/whitespace.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/self-provided/transaction_details_wrapped.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/self-provided/sparkassen.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/self-provided/raphaelm.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/self-provided/raiffeisen-cmi.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/self-provided/overly_long_details.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/self-provided/multiline.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/self-provided/mt942.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/self-provided/malformed_details.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/self-provided/long_statement_number.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/self-provided/invalid_statement.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/self-provided/gv_codes.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/self-provided/february_30.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/self-provided/empty_non_swift.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/self-provided/details_60-63.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/self-provided/account_identification.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/sberbank/171011_01234945.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/requirements.txt | Removes legacy test requirements file. |
| mt940_tests/mBank/with_newline_in_tnr.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/mBank/mt942.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/mBank/mt940.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/jejik/triodos.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/jejik/sns.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/jejik/rabobank.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/jejik/rabobank-iban.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/jejik/postfinance.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/jejik/knab.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/jejik/ing.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/jejik/generic.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/jejik/abnamro.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/cmxl/statement_number.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/cmxl/statement_line.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/cmxl/statement_details.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/cmxl/reference.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/cmxl/mt940.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/cmxl/closing_balance.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/cmxl/available_balance.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/cmxl/account_identification_legacy.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/cmxl/account_identification_iban.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/cmxl/account_balance_debit.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/cmxl/account_balance_credit.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/betterplace/with_binary_character.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/betterplace/sepa_snippet.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/betterplace/sepa_snippet_broken.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/betterplace/sepa_mt9401.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/betterplace/missing_crlf_at_end.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/betterplace/empty_line.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/betterplace/empty_entry_date.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/betterplace/empty_86.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/betterplace/currency_in_25.yml | Updates expected fixture output to match parser changes. |
| mt940_tests/betterplace/amount_formats.yml | Updates expected fixture output to match parser changes. |
| MANIFEST.in | Removes legacy sdist manifest configuration. |
| Makefile | Removes legacy make-based dev commands. |
| lefthook.yml | Adds lefthook configuration to replace pre-commit. |
| docs/requirements.txt | Updates docs requirements to new editable extra syntax. |
| docs/mt940.utils.rst | Adjusts Sphinx directive ordering. |
| docs/mt940.tags.rst | Adjusts Sphinx directive ordering. |
| docs/mt940.rst | Adjusts Sphinx directive ordering. |
| docs/mt940.processors.rst | Adjusts Sphinx directive ordering. |
| docs/mt940.parser.rst | Adjusts Sphinx directive ordering. |
| docs/mt940.models.rst | Adjusts Sphinx directive ordering. |
| docs/mt940.json.rst | Adjusts Sphinx directive ordering. |
| docs/installation.rst | Updates installation instructions to pip/uv workflows. |
| docs/index.rst | Replaces README.rst include with a brief intro + README link. |
| docs/conf.py | Switches theme to furo and modernizes intersphinx config. |
| CONTRIBUTING.rst | Updates contribution workflow instructions to uv/tox. |
| .readthedocs.yaml | Updates RTD build image and Python version. |
| .pre-commit-config.yaml | Removes legacy pre-commit configuration. |
| .gitignore | Updates ignored caches, uv lockfile, and local scratch artifacts. |
| .github/workflows/publish.yml | Adds trusted-publishing flow (build + publish) and runs CI as a release gate. |
| .github/workflows/main.yml | Removes legacy single CI workflow. |
| .github/workflows/codeql.yml | Adds CodeQL analysis workflow. |
| .github/workflows/ci.yml | Adds split CI jobs for tests/coverage/lint/type-check/docs. |
| .github/dependabot.yml | Retargets Dependabot updates to develop. |
Comments suppressed due to low confidence (1)
mt940/processors.py:250
- _parse_segments() currently assumes segment IDs are always exactly 2 characters (it always uses
segment[2:]and reads only two chars after '?'). That means 3-character segment IDs like28D(used elsewhere in this PR and in tests) can never be produced, so downstream logic (e.g._process_segmentsspecial-casing28D) is effectively dead and the issue-109 handling can be incorrect for real inputs.
# If there aren't enough characters left to form a segment type,
# exit the loop.
if index + 2 >= len(detail_str):
break
# Finalize the current segment. If a segment type exists, skip the
# first two header characters.
tmp[segment_type] = segment if not segment_type else segment[2:]
# Extract the new segment type from the following two characters.
segment_type = detail_str[index + 1] + detail_str[index + 2]
# Reset the segment accumulator for the next segment.
segment = ''
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Modernizes the package to the same convention set used in
numpy-stl: uv-based, ruff, fully type-checked, nosetup.py.Highlights
Build / packaging
setuptools→uv_build; dropped the spurioussetuptoolsruntime dep (no runtime deps remain).requires-python >=3.10,README.md, cleaned classifiers.importlib.metadata(and now exported asmt940.__version__).pyproject.toml; mypy strict; added pyrefly; coverage[paths]mapping; dev tools in a PEP 735[dependency-groups].Tooling / CI
tox.toml→tox.ini(py310–313, lint, pyright, mypy, pyrefly, docs, coverage, codespell, repo-review) withtox-gh-actionsand a coverage-combine step enforcing 100%.ci.yml+publish.yml(PyPI Trusted Publishing) +codeql.yml; dependabot targetsdevelop.Docs
README.rst→README.md(absolute URLs); furo theme; Read the Docs → ubuntu-24.04 / Python 3.13.Cleanup
MANIFEST.in,Makefile,pytest.ini,tox.toml, duplicaterequirements*.txt; gitignoreuv.lock, caches, and local scratch files.Also includes the in-progress #109 segment fix (strip trailing BIC/IBAN identifiers) as its own commit, plus minor type/lint fixes in
models.py,tags.py,test_sta_parsing.py.Verification
Full
toxgreen across py310–313 and every env: ruff check + format, pytest (143 passed, 100% coverage), mypy(strict), pyright, pyrefly, codespell, docs (furo,-W), repo-review.Follow-ups (maintainer)
lefthook installlocally to wire up the new git hooks.mt-940+ a GitHubpypienvironment before the nextv*tag sopublish.ymlworks.