Traj2d.timestep is failing

[knutfrode]

I added an xfailing test for this:
https://github.com/OpenDrift/trajan/blob/main/tests/test_read_sfy.py#L33

trajan/trajan/traj2d.py

Line 30 in dc6f354

def timestep(self, average=np.nanmedian):

Traj2d.timestep uses
np.diff(self.ds.time, axis=1)
but we should instead use dimension names here (the failing SFY dataset has only one axis).

Also, would it not be better if Traj1d.timestep and Traj2d.timestep returned timedeltas instead of seconds?

[gauteh]

Yes. I also think we should return a multi dimensional array for time steps on 2d datasets.

[knutfrode]

Ok, I will implement that.

I find those time-things in Python really messy, but I guess we have no better alternative than to use numpy.timedelta64 here ?
I.e. an Xarray DataArray of numpy.timedelta64.

[gauteh]

pd.Timedelta, e.g. from pd.to_timedelta are the best. They serialize to np.timedelta64 internally in xarray. The pandas functions for timedelta and datetime are the best of all, can take anything (single value, array) and can convert to anything. They usually figure out the right thing automatically.

[knutfrode]

In fact, we already have methods time_to_next for exactly this....
I guess timestep was simply made as a shorthand to get a single timestep (either the constant one, or a mean of the varying timesteps).

trajan/trajan/traj2d.py

Line 38 in dc6f354

def time_to_next(self):

trajan/trajan/traj1d.py

Line 42 in dc6f354

def time_to_next(self):

However, time_to_next() fails for the SFY dataset (bug32), since the method is hardcoded for having a dimension named obs
Thus that should be fixed first, but again raises a more fundamental issue/question:

We have self.obsdim and self.timedim - but in fact those are often not dimensions at all, but rather variables.
Thus I think we need a more precise terminology here, i.e. separating clearly between:

• dimensions
• coordinates
• coordinate variables
• regular variables

[gauteh]

I think this is not relevant anymore, closing for now