Skip to content

Add synthetic financial ledger fixture with accounting negatives (#496) - #503

Open
muhammad-muneeb3 wants to merge 1 commit into
FreshCode-Org:mainfrom
muhammad-muneeb3:issue-496-financial-ledger
Open

muhammad-muneeb3 wants to merge 1 commit into
FreshCode-Org:mainfrom
muhammad-muneeb3:issue-496-financial-ledger

Conversation

@muhammad-muneeb3

Copy link
Copy Markdown

What changed?

  • Added a new synthetic test fixture tests/fixtures/financial_ledger.csv and its accompanying expectations file tests/fixtures/financial_ledger.expectations.json[cite: 1, 3].
  • Extended parse_currency_parts in src/freshdata/semantic/experts.py to identify and parse parenthesized accounting negative values (e.g., (1,250.00)) as valid negative numeric values[cite: 1, 2].
  • Added financial identifier checks via _MONEY_NAME in src/freshdata/engine/context.py and src/freshdata/semantic/context.py so amount and balance columns are not misclassified as id or text roles due to high cardinality[cite: 2].
  • Registered financial_ledger in tests/expectations.py, generated the golden snapshot, and added targeted test coverage in tests/test_currency_locale.py[cite: 1, 2, 3].

Why?

Ensures FreshData robustly handles localized financial conventions—specifically parenthesized accounting negatives and currency formats—without regressing semantic detection or data cleaning pipelines[cite: 1].

Fixes #496[cite: 1]

How was it tested?

  • New unit tests added in tests/
  • Ran fast CI lane locally: pytest -m "not online and not large"
  • Ran linting and type checks: ruff check . and mypy src/freshdata.

Targeted test command run locally:

pytest tests/test_currency_locale.py tests/test_realworld.py -k financial_ledger tests/test_golden.py -k financial_ledger tests/test_experts_guard_mutants.py -k currency --no-cov

## Any performance impact?

<!-- Will this change affect wall-clock runtime, memory allocation, or startup time? If yes, provide timings. -->

- [x] None / negligible
- [ ] Measured with `benchmarks/bench.py` (details below):

## Any compatibility concerns?

<!-- Does this change public API signatures, default behavior, or supported Python/pandas versions? -->

- [x] None / fully backward-compatible
- [ ] Deprecation or behavior change documented below:

## Documentation updated?

<!-- If user-facing behavior changed, did you update docs/, examples/, or CHANGELOG.md? -->

- [ ] Documentation updated in `docs/`
- [ ] Examples verified or updated
- [x] Note added under `[Unreleased]` in `CHANGELOG.md`

@coderabbitai

coderabbitai Bot commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 41596f35-2394-4dd1-9392-e3bb3f14b63f


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add synthetic financial ledger fixture with accounting negatives and currency conventions

1 participant