Sanitiser update and fix

[dylanjmcconnell]

Some of the cells in v7.8 of the workbook have a new type of within-cell footnotes. In addition, previous sanitisation of in-cell comments also silently mis-handles some comments (this effects v7.8 but also some older versions too). This PR address both of these by updating and fixing sanitisers.py

New footnote style

There are now square-bracketed footnotes, immediately next to a value (e.g. 750[footnote14]). This pattern not captured by any of the existing sanitisers - so a new one is added as follows:

def _remove_series_bracketed_footnotes(str):
    return series.str.replace(r"\[footnote\s*\d+\]", "", regex=True)

(and the function added to the list of santisers to run)

Mis-handled santisation

_remove_series_notes_after_values is intended to handle the common 123 (some note) shape. But the existing regex patterns stops at a $ sign (and also : and probably others) - so some things are silently not-sanitised. For example:

1,749.5 (ElectraNet has advised that approximately $23 million of this amount relates to approved early works costs ...)

Is returned as:

1749.5$23 million of this amount relates to approved early works costs…

(i.e. the bit from the parenthesis to the $ is removed, but not anything after the $ sign)

Have up updated the santiser to capture broader more generic pattern, from:

r"^([0-9\.]+)\s+(?:(\([\w\s\.\<\=\-\/\,]+\)?\s?)+)"

to:

r"^([0-9\.]+)\s+\(.*$"

This is a much more generic regex - i.e. basically capture anything following whitespace and opening parenthesis - i.e. anything after <number><whitespace>(. Not just the whitelist of characters in the original (not sure if there was a reason for that original white list?).

I did regenerate the example outputs to see if there were negative side effects of this .. I did spot two patterns that are not footnotes, and captured by this (but also - previously there were incorrectly captured as footnotes, and maybe are a different category of problem / issue):

1. gas_system_properties_pipelines: 350 (Summer) / 362 (Winter) - now sanitises as 350 with new regex (and previously as 350362 (Winter))
2. rez_augmentation_options_VIC table: 4600 (V8: 3,000) - now sanitises as 4000 with new regex (and previously as 4600: 3000))

So this PR would make these values go from "incorrectly treated as comments and poorly sanitised", to "incorrectly treated as comments and slightly better sanitised". The reason I say "slightly better" is mainly because it means the the rest of the table has the consistent dyptes (rather the float and str in same col). Perhaps this is better dealt with as a separate issue, if at all (... pretty niche issue at the end of the day, I think). But open to other suggestions.

I did make a table with current output, vs updated output and original text (.. mainly because was hard to spot changes in the git diffs). I've put it here incase it's handy for anyone else to look at too: Comment changes and fixes for workbook 7.8.

Tests:

Added some basic tests for the new / updated functions intests/test_sanitisers.py:

• test_remove_series_notes_after_values_with_special_characters
• test_remove_series_bracketed_footnotes.

File changes:

• src/isp_workbook_parser/sanitisers.py
• tests/test_sanitisers.py

And then a handful of output csvs in the example_output csv folder

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines	Coverage Δ
src/isp_workbook_parser/sanitisers.py	100.00% <100.00%> (ø)

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[nick-gorman]

Looking at the changes to the output this seems to be strictly an improvement on current data. I've just got a few questions/comments which might make it more robust on future datasets. Up to you if you think they are worth implementing.

[nick-gorman]

Just a thought, should the subsequent regex replacements have the same fix applied to them as well, so they don't also stop at a $?

[nick-gorman]

Would it be a bit more robust if we move the _remove_series_bracketed_footnotes up the pipeline to before _remove_series_notes_after_values, that way a bracketed footnote, say after the first number can't intefer with _remove_series_notes_after_values from firing?? Also moving _remove_series_trailing_footnotes to before _remove_series_notes_after_values could also guard a trailing footnote from being caught as second value.

[nick-gorman]

Also maybe we don't want the bracketed footnote firing on column headers, but if we didn't mind that we could move its regex to custom_string_replacements.py

[dylanjmcconnell]

Thanks Nick!

fyi change the pipeline ordering as you suggest (.. in part because it was straightforward).
'
I didn't apply the same regex fix to the other patterns (.. i.e. so they also don't stop at a $ etc). Maybe overly conservative - (e.g. might accidentally let something through)..

But also not entirely theoretical - just noticed that the update would impact some of the carbon budget ranges ("28-33% reduction" would end up as 28, and a float rather than string) .. though that is kind of another of a problem - a range in a cell 🫠 .

Anyway I think best leave that for now.