Performance improvements

[davidhassell]

Fixes #201
Fixes #202
Fixes #203
Fixes #204

[davidhassell]

Hi @sadielbartholomew,

This is another rambling old PR (106 files changed, this time), but most changes are either style, linting, or the implementation of the new API throughout the library.

This PR basically brings in the cfdm changes from NCAS-CMS/cfdm#138; adds some PP related optimisations; and looks a bit ahead to CF-1.9 (with fielddomainlist.py)

When reviewing, you might search for 3.9.0 in the code, or I might recommend starting with:

cf/core/constructs.py
cf/constructs.py
cf/mixin/container.py
cf/mixin/*.py
cf/mixin/fielddomain.py
cf/mixin/fielddomainlist.py
cf/field.py
cf/fieldlist.py
cf/read_write/um/umread.py [*]

[*] Interacts with field.py for some short-circuiting optimisations when setting metadata constructs on a field.

You might get away with just that, but I'm sure that you could also treat these files as spring board to other things.

Thanks.

[sadielbartholomew]

Thanks for another helpful review guide @davidhassell. Once my append mode work is out for review I'll be straight onto this...

[sadielbartholomew]

I am mid-review and will finish up on Sunday evening, but here are some initial minor comments.

[sadielbartholomew]

I've completed my review and I am very happy with this: all associated issues have been addressed satisfactorily and notably the performance has indeed improved for both netCDF and PP selection reads, as tested with similar code snippet examples as provided in those issues and backed up by shorter times to run a sample of the tests (due to seg faults I can't currently run all of them at once locally). Great work. Users will really appreciate the performance gains 🚀

I'v registered some minor comments in-line for your consideration. As a final note I see you've updated the docs here and otherwise all changes look to have been included, but I can't see FieldDomainList or FieldDomain from a search in the docs generated from this branch, so they might need adding to the class listing unless there's a reason you have left them out?

[davidhassell]

As a final note I see you've updated the docs here and otherwise all changes look to have been included, but I can't see FieldDomainList or FieldDomain from a search in the docs generated from this branch, so they might need adding to the class listing unless there's a reason you have left them out?

These were left out along with all of the other mixin classes. Do you think that the mixins should be in docs? I think I left them out because they don't work as stand alone objects and don't form part of the public API

[sadielbartholomew]

These were left out along with all of the other mixin classes. Do you think that the mixins should be in docs? I think I left them out because they don't work as stand alone objects and don't form part of the public API

Ah, that makes sense. Thanks for the clarification. I see, I hadn't noticed or thought about the mixin classes in relation to the docs before but on reflection it sounds like a good idea to keep them out of the docs for the reasons you state. Though maybe, in future, we could somehow include them in a 'development' section or something along those lines for full transparency for the small number of people it may be useful for.

[davidhassell]

Though maybe, in future, we could somehow include them in a 'development' section or something along those lines for full transparency for the small number of people it may be useful for.

Good idea - let's put up an issue for it.

[sadielbartholomew]

Excellent, all feedback has been addressed well, most notably that cf.example_fields() works without arguments.

The tests are passing locally (with no seg faulting 🎉 ) and with much improvement in speed. I am not sure why the Actions jobs haven't been triggered and run, but we will test those as part of the release checklist so that can and will be done shortly and therefore it's OK.

Please go ahead and merge!

P.S. When we update the changelog for the imminent releases of for both cf-python and cfdm, I think it would be good to explicitly highlight that performance (generally or specifically) should have improved significantly.

[davidhassell]

Thanks for review, @sadielbartholomew, merging now.