180 cleanup for 195

[CBROWN-ONS]

Description

This issue handles points 3, 4, 5, 6 from #180

Does not fully fix any issues, only partially #180
This issue should be marked as closed once #195 is merged.

Motivation and Context

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

How Has This Been Tested?

Test configuration details:

• OS: Windows 10
• Python version: Python 3.9.16
• Java version: N/A
• Python management system: MiniConda

Advice for reviewer

Checklist:

• My code follows the intended structure of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published in downstream modules

Additional comments

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.18%. Comparing base (7bb4863) to head (4886a42).

Additional details and impacted files

@@           Coverage Diff           @@
##              dev     #254   +/-   ##
=======================================
  Coverage   98.17%   98.18%           
=======================================
  Files          21       21           
  Lines        1915     1924    +9     
=======================================
+ Hits         1880     1889    +9     
  Misses         35       35           

Flag	Coverage Δ
unittests	98.18% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[r-leyshon]

Hi @CBROWN-ONS , I've finished up with the review. A couple of queries that I've put in the review comments and something to check with the person raising the initial issue, believe this was @ethan-moss .

Also, I couldn't see an implementation for point 5 in #180 :

"5. For _add_validation_row() add defence to ensure message is only either {"warning", "error"}".

It may be handled in a different PR, so worth checking.

Also please note, I have patched 2 of the CI problems in this branch so do go ahead and check you're happy with that. It was the expired feed fixture thing and then a slight tweak in the requirements file to ensure dask comes with its dataframe dependency.

[CBROWN-ONS]

Hi @r-leyshon . Point 5 as addressed in you review comment above is fixed in this PR. More specifically, this line of code.