Skip to content

Update column (SA-674)#10

Merged
dstewartons merged 5 commits into
mainfrom
SA-674/soc-lookup-csv-compatibilty
May 15, 2026
Merged

Update column (SA-674)#10
dstewartons merged 5 commits into
mainfrom
SA-674/soc-lookup-csv-compatibilty

Conversation

@dstewartons

@dstewartons dstewartons commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

📌 Pull Request Template

Please complete all sections

✨ Summary

Align SIC rephrase example data and SICRephraseLookup with SA-674 naming: CSV column reviewed_descriptionrephrased_description, and the same name on lookup() return values / process_json access.

Breaking change (library API): SICRephraseLookup.lookup previously returned reviewed_description, it now returns rephrased_description. Downstream code that read the old key must switch.

📜 Changes Introduced

  • src/industrial_classification/data/example_rephrased_sic_data.csv — header: final column renamed to rephrased_description.

  • src/industrial_classification/lookup/sic_lookup.pySICRephraseLookup: read that column; dict key rephrased_description; docstring tweak.

  • docs/example_data.md — table header updated.

  • src/industrial_classification/lookup/sic_lookup_example.py — comment aligned.

  • Feature implementation (feat:) / bug fix (fix:) / refactoring (chore:) / documentation (docs:) / testing (test:) — chore / data contract alignment (plus docs)

  • Updates to tests and/or documentation — docs + example module comment, tests unchanged (none covered SICRephraseLookup directly)

  • Terraform changes (if applicable) — N/A

✅ Checklist

Please confirm you've completed these checks before requesting a review.

Verified on 2026-05-01 from repo root with poetry install (installs the package; required for imports) then make check-python-nofix and poetry run pytest -q.

  • Code is formatted using Blackpoetry run black . --check (via make check-python-nofix)
  • Imports are sorted using isortpoetry run isort . --check (via same target)
  • Code passes linting with Ruff, Pylint, and Mypy — all passed (same target)
  • Security checks pass using Banditpoetry run bandit -r src/industrial_classification (same target)
  • API and Unit tests are written and pass using pytest10 passed
  • Terraform files (if applicable) — N/A
  • DocStrings follow Google-style — no new docstrings required; existing style preserved
  • Documentation has been updated if needed — docs/example_data.md

🔍 How to Test

cd sic-classification-library
poetry install
make check-python-nofix
poetry run pytest -q

Optional: run sic_lookup_example.py from repo root and confirm printed SICRephraseLookup.lookup output includes rephrased_description.

@Tom-Owen-ONS

Copy link
Copy Markdown

Hey, I can see the column name being updated, but the SICRephraseLookup class is still looking for a "reviewed_description" column here, so src/industrial_classification/lookup/sic_lookup_example.py fails for me. Perhaps the change has been made but not pushed yet?

- keep lookup responses stable while tightening code path for example and configured data
@dstewartons

Copy link
Copy Markdown
Contributor Author

Hey, I can see the column name being updated, but the SICRephraseLookup class is still looking for a "reviewed_description" column here, so src/industrial_classification/lookup/sic_lookup_example.py fails for me. Perhaps the change has been made but not pushed yet?

@Tom-Owen-ONS, yes some missing commits, this should work now

@Tom-Owen-ONS Tom-Owen-ONS left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works great now, thanks for updating :)

@dstewartons dstewartons merged commit 2f2592d into main May 15, 2026
5 checks passed
@dstewartons dstewartons deleted the SA-674/soc-lookup-csv-compatibilty branch May 15, 2026 10:32
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.

2 participants