GitLab

This looks good. So, do era5, hycom, and wwiii all use 3-hour binning? (So using timedelta(hours=3) ensures that the request includes one time bin?)

hycom uses 3-hour, and i believe the other two use 1-hour binning. I chose 3-hour as its the shortest time frame that still covers at least one time bin for all data sources

Ok. So for WWIII and ERA5, two or three time bins will be downloaded and then averaged?

yes, unless the user specifies a smaller time frame with the 'start' and 'end' kwargs

Ok, we may want to find a better solution in the future, but it will do for now. I think this needs to be explicitly described in the methods docstrings. I will add this now.

@matt_s extra question: is the time averaging done at fetching or loading stage? This is not clear from the doc strings.

It would be helpful if you could add more details to the fetch/loading methods of hycom, era5, and wwiii, specfically addressing the aspect of time averagning

@kirsebom the fetching is always done in 24-hour chunks, averaging is done in the ocean module pipeline after the loading step. how it works is the load functions return lat, lon, depth and time, and then averaging is done during ocean initialization using the reshape_3D function in data_util. I mentioned this briefly in the ocean module docstrings, maybe I should elaborate on it a bit more?

Oh, I had overlooked the docstring of the ocean class, sorry. The explanation given there is good enough, provided we remove the time argument and stick with only using start and end.

@matt_s , next time please implement changes in a separate branch, then create a merge request into the main development branch. I know this can feel a little tedious for minor fixes, but it's good practice and avoids situations like where CI tests are failing on the main development branch. Cheers, Oliver

I've created a new branch 'matt_branch' and will merge passing changes from here to the development branch in the future

merged

mentioned in commit 187408a8