smerckel / dbdreader

A reader for binary data files created by Slocum ocean gliders (AUVs)
GNU General Public License v3.0
17 stars 14 forks source link

return type instability #19

Closed jklymak closed 1 year ago

jklymak commented 1 year ago

I suppose you would do

t,v,srcs = data[parameter]

which would then turn into

(t,v), srcs = data[parameter]

Originally posted by @smerckel in https://github.com/smerckel/dbdreader/issues/14#issuecomment-1505497875

I think #14 introduced a pretty strong breaking change? Does data[parameter] return a tuple now? This looks to have broken https://github.com/c-proof/pyglider but I haven't investigated thoroughly.

smerckel commented 1 year ago

14 Introduced a feature asked for by BODC to have each data point labelled with the source file. This feature is not something I would use in my normal work flow, and presumably most users will not use it. Therefore the default setting is that is not used. To use this feature, it is required to use MultiDBD's get() method with the keyword include_source set to True. If pyglider is broken, I guess it is not because of this feature.

The return types of all get* methods have been harmonised though, as this has evolved in an inconsistent way over time. Essentially, if one parameter is asked for, a tuple with time and values vectors is returned. If more than one parameter is asked for, then a list of such tuples is returned. Previously, some methods would return a tuple, others would return an ndarray.

While writing this, I had a look at the pyglider code. I will have a look at it tomorrow to see where it breaks and how best to fix it.

smerckel commented 1 year ago

Hi @jklymak , I had a look at the pyglider code base and noticed that the code would break a one position. Here you would put the return value of MultiDBD.get_sync() in data, and subsequently modify the data structure by removing the first element and moving the second element to another place. This obviously does not work for tuples. I submitted a pull request to fix the issue in your code by converting the returned tuple directly into a list, and all works again.

jklymak commented 1 year ago

Great that was easier than I feared. Thanks!