Hi @davidhassell, all pushed except the featureType approach implementation which led to some hanging for some reason so I am revising. Won't be long and I will commit and push that.

Please can you test this on your env and let me know if you are still seeing #69 (comment)? And if you like you could review the commits made since your initial review, etc.? Thanks.

Opening and closing to trigger the Actions jobs now...

Codecov Report

Merging #69 (ecc8cc5) into master (ddf7d16) will increase coverage by 0.28%.
The diff coverage is 84.77%.

@@            Coverage Diff             @@
##           master      #69      +/-   ##
==========================================
+ Coverage   88.54%   88.82%   +0.28%     
==========================================
  Files         101      101              
  Lines       10435    10528      +93     
==========================================
+ Hits         9239     9350     +111     
+ Misses       1196     1178      -18     

Flags with carried forward coverage won't be shown. Click here to find out 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 ddf7d16...ecc8cc5. Read the comment docs.

(Previous feedback discussed and further feedback raised externally to GitHub, on Slack.)

The CI jobs and Codacy are happy! Saying that, they didn't pick up on one issue I noticed as soon as I started those jobs (naturally), where a flag variable was defined in slightly the wrong place. The following commit fixes that.

All feedback should now be addressed, what do you think @davidhassell ?

I won't merge this yet, though, since I have a few minor issues to resolve that only show up in the equivalent cf-python test.

#69 (comment) is not in fact an issue in the code, I have now realised, since the cf example fields, n=5 and n=2 are aggregatable and indeed aggregate down to one field:

>>> import cf
>>> b = cf.example_field(2)
>>> print(b)
Field: air_potential_temperature (ncvar%air_potential_temperature)
------------------------------------------------------------------
Data            : air_potential_temperature(time(36), latitude(5), longitude(8)) K
Cell methods    : area: mean
Dimension coords: time(36) = [1959-12-16 12:00:00, ..., 1962-11-16 00:00:00]
                : latitude(5) = [-75.0, ..., 75.0] degrees_north
                : longitude(8) = [22.5, ..., 337.5] degrees_east
                : air_pressure(1) = [850.0] hPa
>>> c = cf.example_field(5)
>>> print(c)
Field: air_potential_temperature (ncvar%air_potential_temperature)
------------------------------------------------------------------
Data            : air_potential_temperature(time(118), latitude(5), longitude(8)) K
Cell methods    : area: mean
Dimension coords: time(118) = [1959-01-01 06:00:00, ..., 1959-02-28 18:00:00]
                : latitude(5) = [-75.0, ..., 75.0] degrees_north
                : longitude(8) = [22.5, ..., 337.5] degrees_east
                : air_pressure(1) = [850.0] hPa
>>> c.equals(b, verbose=-1)
Data: Different shapes: (118, 5, 8) != (36, 5, 8)
Field: Different data
False
>>> a = cf.aggregate([b, c])
>>> a
[<CF Field: air_potential_temperature(time(154), latitude(5), longitude(8)) K>]

which (noting the 154 rather than 118 time points) is the field indeed set in the file post-append, so the test just needs adapting to test that n=5 doesn't change the length of the overall field list. I am about to push a commit that does that over on NCAS-CMS/cf-python#213.

(Discussing via Slack and investigating some slowness emerging in another test_read_write test before we are confident to that this is good to merge.)

#69 (comment) turned out to be unrelated to the PR code changeset and has been addressed in
87c8c5a, with all credit to @davidhassell 😃 Now we should be ready to go...

All tests pass for me, and in the expected amount of time (apart from a pep8 white space error in test_read_write.py - but that's probably just me, as I had to fix a merge conflict in there).

Good work!

Opening-closing to check the Actions jobs still pass...

🎉 🎈 Good to merge on all counts! Thanks for all of your help with this @davidhassell, we got there in the end... Now to the cf-python equivalent; review to be opened on #NCAS-CMS/cf-python#213 very soon.