Fix for cf.aggregate failure when a datum or coordinate conversion parameter has an array value

[davidhassell]

Fixes: #230

[codecov-commenter]

Codecov Report

Merging #231 (d979221) into master (25cfacf) will decrease coverage by 0.75%.
The diff coverage is n/a.

❗ Current head d979221 differs from pull request most recent head d37d36c. Consider uploading reports for the commit d37d36c to get more accurate results

@@            Coverage Diff             @@
##           master     #231      +/-   ##
==========================================
- Coverage   74.52%   73.77%   -0.74%     
==========================================
  Files          80       85       +5     
  Lines       18789    19179     +390     
==========================================
+ Hits        14001    14148     +147     
- Misses       4788     5031     +243     

Flag	Coverage Δ
unittests	73.77% <ø> (-0.74%)	⬇️

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

Impacted Files	Coverage Δ
main/cf/examplefield.py	91.67% <0.00%> (-8.33%)	⬇️
main/cf/field.py	62.64% <0.00%> (-5.64%)	⬇️
main/cf/flags.py	66.93% <0.00%> (-5.51%)	⬇️
main/cf/fieldlist.py	85.72% <0.00%> (-3.23%)	⬇️
main/cf/aggregate.py	80.58% <0.00%> (-1.69%)	⬇️
main/cf/coordinatereference.py	73.94% <0.00%> (-1.31%)	⬇️
main/cf/cellmeasure.py	62.00% <0.00%> (-0.74%)	⬇️
main/cf/mixin/properties.py	74.43% <0.00%> (-0.57%)	⬇️
main/cf/mixin/coordinate.py	95.63% <0.00%> (-0.21%)	⬇️
main/cf/data/data.py	78.80% <0.00%> (-0.16%)	⬇️
... and 36 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d9cde28...d37d36c. Read the comment docs.

[sadielbartholomew]

This seems to work well though:

• we could do with a corresponding test, ideally bundled with this PR, to isolate the original aggregate failure (as also indicated by the decrease in coverage on the Codecov report);
• given that the solution relies on the recursive function _totuple, I wonder if there may be a negative effect on performance (it's difficult to check that without a test to expose the case at hand, though) do you have any ideas about the potential hit in speed and/or the specifics and nicheness of the case that might influence how consequential it could be to users?

[davidhassell]

Thanks, Sadie.

I'm not too worried about performance. The recursive nature is "belt and braces" - there is no use case for multidimensional parameter values and none are specified by the conventions - so it shouldn't be a an issue. If it becomes so we can review it then.

Always a good idea to add a test! I'll do that ...

[sadielbartholomew]

Thanks David.

The recursive nature is "belt and braces" - there is no use case for multidimensional parameter values and none are specified by the conventions - so it shouldn't be a an issue. If it becomes so we can review it then.

Fair enough, that's all good by me.

Always a good idea to add a test! I'll do that ...

Excellent. Please merge away once the test is added (or tag me to look at the update if you prefer, I am happy to do so).

[davidhassell]

Test added, Pretty straight forward, so merging.