scitools-incubator / iris-ugrid Goto Github PK

#4 is re-implementing some prototype code for Iris, but has left out some specialised handling for the slightly different format output by XIOS, which is recorded below should the knowledge prove useful in future.

There is a definite difference in XIOS structure vs 'normal' UGRID files, and specialised handling therefore seemed necessary, but it was found that just using the 'standard' code still achieved the desired values in exclude_vars (probably a happy accident!).

        temp_xios_fix = kwargs.pop("temp_xios_fix", False)
        if not temp_xios_fix:
            # This way *ought* to work, but maybe problems with the test file ?
            for mesh in meshes.values():
                mesh_var = dataset.variables[mesh.mesh_name]
                for attr in mesh_var.ncattrs():
                    if attr in _UGRID_LINK_PROPERTIES:
                        exclude_vars.extend(mesh_var.getncattr(attr).split())
        else:
            # A crude and XIOS-specific alternative ..
            exclude_vars += [
                name
                for name in dataset.variables.keys()
                if any(name.startswith(meshname) for meshname in meshes.keys())
            ]

With a hook in the repo for testing on a PR.
Implement via pytest.
See e.g. https://github.com/SciTools/tephi

As noted in the code comments, the use of the mdtol argument is slightly misleading :
It only labels output cells as missing where there is insufficient overlap with the boundary of the source grid,
- it does not support masked source points.

This can definitely be done, but maybe at a computational cost.
There is a crude implementation in Iris here.

Populate README.md in line with other SciTools and SciTools-incubator projects.

• Include licence line(s) (#1).
• Include the intended relationship between the SciTools/iris and this repo
  • Functionality relating specifically to unstructured data belongs in this repo
  • Functionality that is generic should belong in iris where possible
  • If functionality here requires supporting changes in iris, those changes should be generic and not make specific reference to unstructured data (excepting narrative such as pull request comments).
• While it exists, mention that this repo actually depends on the iris ng-vat_v2 feature branch rather than any iris releases

Suggest that PEP440 versioning is used.

Base on other SciTools repos (tephi as usual being the flagship example).

Known requirements introduced by #4:

• run
  • iris
  • gridded
• test
  • iris-test-data

Not sure how to reconcile the fact that we need special commit hashes for iris and iris-test-data (see #7 for details). I notice Iris doesn't actually mention iris-test-data in its test requirements.

• Decide on a licence
• Create licence file
• Add licence line(s) to README.md

Some retrospective ideas following #10
Probably not worth pursuing now : until this progresses to be a stable, supported public API (which might never happen).

All possible, to be considered/investigated :

• ? add a crs option to the regrid.MeshInfo() constructor, to match regrid.GridInfo
• ? rename the regrid.GridInfo() args 'lats', 'lons' etc to 'x'/ 'y', to suit the generality of the crs usage
• ? make MeshInfo.size and GridInfo.size private, as purpose to the user is not really clear
  • (or) just document them fully

Replicate the labels used in the tephi repo - colours and names.

Like #19 , this may not be needed yet.
However, unlike that, this will be needed at some point, if the regrid code becomes part of a stable release.

Basically, bring testing into line with a #17.
At present, we are lacking some specific aspects ..

• tests for additional, optional args in MeshInfo().__init__ :
  • node_start_index, elem_start_index, areas
  • testing for mathematical results is not necessary : that would be testing ESMF rather than iris-ugrid
  • ( ... should test the resulting object properties instead )
• tests for additional, optional args in GridInfo().__init__ :
  • for crs, actual behaviour/results testing is needed
  • for circular / areas, previous note applies : just check properties of created object
• Regridder.regrid needs test for mdtol argument.
  • needs to check actual functional correctness

Need a fix to keep Cirrus working, see SciTools-incubator/iris-esmf-regrid#29 for details.

Base on those included in other SciTools repos (tephi as usual being the flagship example).

• Will need to custom install iris (this commit) and iris-test-data (this commit).
  See how iris' Travis does iris-test-data for details.
• Travis should reference a requirements txt file - see #6.

Enforce the following coding standard:

• Use pytest
• Use pytest-xdist
• Use coverage
• Maintain flake8 compliance
• Maintain black compliance
• Configure pre-commit git hooks, as per iris

While we have currently replicated many popular repositories by having the source code in a first level directory (/iris_ugrid/), it is actually considered better practice to contain source code within an extra directory layer, e.g. /lib/iris_ugrid/ - the structure already used in Iris and Cartopy.

See #4 (comment) and pypa/sampleproject#104 for specifics, and the full discussion on #4 may also be of interest.

The iris-ugrid is missing its LICENSE file.

I think this is a general oversight, but I'd vote for this repo to be BSD-3 Clause, rather than LGPL.

Note that, we are in the process of moving iris away from LGPL to BSD-3 Clause.

This would invalidate any source code preamble headers.

Other SciTools projects as template (note that only some of them currently use RTD).

• Adhere to the following numpy doc-string standard
• Create a template sphinx docs directory

We'll want a "standard arrangement" structure + some rules, to organise our tests.

Can be analagous to the Iris ones.
In-particular // at-least :

• tests for all public API components
• unit-tests organised into standard structures by class/method.function, matching the package hiearchy
  • but not really clear how this will look when using pytest instead of unittest ?

In addition/alternative to Iris practices, I suggest :

• use pytest to best advantage (or, work out how to).
• minimise use of Mock, or any implementation-dependent testing
• minimise use of 'rubber-stamp' testing like comparing str or repr forms, or iris.tests.assertCML()
  • i.e. where possible, test of relevant (public) properties instead of a 'total content dump' approach.
• consider division into standard-ci and extended testing (maybe targetted by some future overnight testing strategy?)

TODOs

• add a guide document for testing 'rules' in iris-ugrid
• ensure all existing tests conform
• add check to the definition-of-done
  • ( I would also say, write a runnable test to check this, but I don't think this stuff is machine-enforceable ! )