added TELEMAC support and tests

[tomsail]

added the following components for normalisation:

• is_telemac()
• normalize_telemac
• THALASSA_FORMATS.TELEMAC
• infer_format()
• NORMALIZE_DISPATCHER

Which should address the normalisation for TELEMAC format discussed in #90

I might have added too many elements:

• xarray-selafin to support directly .slf file formats
• Global variable names in order to prepare for a more mainstream formating convention (like UGRID):

EDGE_DIM = "edge"
FACE_DIM = "face"
NODE_DIM = "node"
VERTICAL_DIM = "layer"
CONNECTIVITY = "face_nodes"
VERTICE_DIM = "max_no_vertices"
X_DIM = "lon"
Y_DIM = "lat"

If it too much for one PR, let me know, I'll drop these last changes

[tomsail]

I forgot the .slf file, the PR is not complete

[tomsail]

ok ready for review

[pmav99]

Looks good, kudos. Just some minor stuff are needed

[pmav99]

why comment out?

[tomsail]

good question, I forgot to mention this:
mask_and_scale is an argument that is not available in the xarray-selafin backend but that seems to exists for all other formats (zarr, grib, netcdf). I don't know how to solve this

[pmav99]

mask_and_scale is about applying the add_offset and the scale_factor. It's a netcdf thing, but xarray also applies it to zarr. I don't know if it makes sense in selafin, but even if it doesn't, I would add it as a do-nothing argument. Either that or I would add a **kwargs so that it can be consumed without throwing an exception

[tomsail]

you mean directly in xarray-selafin, correct?

[pmav99]

According to the docs a **kwargs is not allowed:
https://docs.xarray.dev/en/stable/internals/how-to-add-new-backend.html#open-dataset

Furthermore, mask_and_scale may not be supported by all backends: https://docs.xarray.dev/en/stable/generated/xarray.open_dataset.html

Therefore, I guess it's OK to remove it. That being said, if the selafin files can be quantized it might make sense to add support for mask_and_scale in xarray-selafin but that's a different issue.

[pmav99]

This is most probably xarray-selafin related, but why is ikle2 an attribute and not a variable?

[tomsail]

Luc actually shifted ikle2 and ikle3 from coordinates to attributes because it removed useless dimensions.
Here's the commit.
We might want to move them back in the variables at some point I agree.

[pmav99]

I wouldn't call the connectivity useless. Furthermore, it might be that serializing attributes needs more space compared to data variables (lack of compression etc). Haven't tested it though.

[pmav99]

Can you please test test_create_trimesh with selafin, too?

[pmav99]

Please update the README, too.

[tomsail]

I might have forgotten something important to remove the overlapping triangles:

this is what

mesh = api.open_dataset('meshes/global-v0.2.slf')
map = thalassa.plot(mesh.isel(time=0), variable = 'B', show_mesh = True)

gives me :

[tomsail]

sorry my bad. I though dropping elements crossing IDL was integrated in the create_trimesh() function.

mesh = api.open_dataset('meshes/global-v0.2.slf')
mesh_ = thalassa.utils.drop_elements_crossing_idl(mesh)
map = thalassa.plot(mesh_.isel(time=0), variable = 'B', show_mesh = True)

did it

[pmav99]

No it isn't. it has to be done manually. we could add a boolean param for it though. E.g.

thalassa.open_dataset("/path/to/netcdf", drop_idl=True)

but that should be discussed in a different issue.

[pmav99]

I had a second look BTW. I am starting to think that we should remove the xarray-selafin package from the main dependencies.

xarray-selafin is pretty much the same as cfgrib or any other xarray engine. If it is installed, then you can use it. But there is no need to pull it when you install thalassa.

So if a user wants to open a selafin file he should just make sure that both thalassa and xarray-selafin are available in their environment:

pip install thalassa
pip install xarray-selafin

and then use:

import thalassa

ds = thalassa.open_dataset("/path/to/foo.slf", engine="selafin")

and things should just work.

That being said, we should still keep it as a dev dependency because we want to use it for our tests.

[pmav99]

These need to be updated, too:

• Thalassa/thalassa/api.py

  Lines 69 to 70 in b9d977c

  	normalize: Boolean flag indicating whether the dataset should be converted/normalized to the "Thalassa schema".
  	Normalization is currently only supported for ``SCHISM`` and ``ADCIRC`` netcdf files.

• https://github.com/ec-jrc/Thalassa/blob/master/docs/index.md

And if there is a publicly available selafin file, we could add it here, too: https://thalassa.readthedocs.io/en/latest/installation.html