Unidata / netcdf-java

The Unidata netcdf-java library
https://docs.unidata.ucar.edu/netcdf-java/current/userguide/index.html
BSD 3-Clause "New" or "Revised" License
145 stars 70 forks source link

Vertical transform refactor #851

Closed JohnLCaron closed 3 years ago

JohnLCaron commented 3 years ago

VerticalTransforms need to be rethought: 1) they use ma2.Array instead of array.Array. 2) they need a NetcdfDataset in their constructor, but NetcdfDataset needs VerticalTransforms to be constructed. 3) they are not used in the library, are they used by library users? 4) They are only implemented in CF NetcdfDatasets, Grib does not implement. 5) Grib no longer is built on NetcdfDataset.

Deprecate (remove in ver8): 1) VerticalCT 2) ProjectionCT 3) TransformType 4) dataset.CoordinateTransform

Could create a brand new implementation or we could remove that functionality all together.

JohnLCaron commented 3 years ago

Ive started down the path of an alternate implementation in ucar.nc2.geoloc.vertical, to see how much trouble it is.

May end up ripping it out.

Deprecating the old in ver7, and removal in ver8, will happen in any case

JohnLCaron commented 3 years ago

ProjectionCT is removed but Projection is needed and stays.

JohnLCaron commented 3 years ago

See PR #852. Im going to consider this resolved when that PR is in.

Ive attached the new VerticalTransform to the GridCoordinateSystem only, not the dataset.CoordinateSystem. This simplifies constructing both, and fits into the idea of removing functionality from dataset, which is too complex.

Im only implementing new VerticalTransform for those that we have tests files for. Im comparing old and new, but really we dont have an independent test of whether these are correct.

These use ucar.array, and are only constructed on top of a NetcdfDataset (not GribGrid). For now, I just want to preserve existing functionality. As usual, the old implementation is pretty messy.