OceanParcels / Parcels

Main code for Parcels (Probably A Really Computationally Efficient Lagrangian Simulator)
https://www.oceanparcels.org
MIT License
295 stars 136 forks source link

`time_periodic` parameter in `from_xarray` has no effect #1209

Closed VeckoTheGecko closed 2 years ago

VeckoTheGecko commented 2 years ago

Hello. I've been looking through the codebase and found the from_xarray method has a parameter time_periodic that is unused in the logic (despite the docstring indicating that it has a use).

https://github.com/OceanParcels/parcels/blob/32600333a087492c177c10a11767ae45b11ea0b4/parcels/field.py#L480-L513

The previous method from_netcdf packs this variable into kwargs, maybe this was what was intended for the from_xarray method also?

I'm curious whether this is a bug or not 😄 (if it is, +1 for VScode syntax highlighting 😍).

erikvansebille commented 2 years ago

Thanks for spotting this, @VeckoTheGecko. I think that it's indeed a bug that the time_periodic parameter is not packed in kwargs. Feel free to create a PR that implements this (and ideally also test if it indeed works as intended?)

VeckoTheGecko commented 2 years ago

Can do @erikvansebille . I'm currently focussing on other areas of the codebase, but will familiarise myself with the test suite and see if I can get this done within a couple weeks 😄