JuliaStats / TimeSeries.jl

Time series toolkit for Julia
Other
353 stars 69 forks source link

`dropinf` and other filter functions #436

Open iblislin opened 4 years ago

iblislin commented 4 years ago

maybe support a general filter; implement dropnan and dropinf on the top of filter function.

imbrem commented 4 years ago

Oops, I just opened an issue (#456) about this without seeing it. If you want I can do a PR. Should we also do filter!, or should we strive to keep everything immutable? My justification for why filter! would work even if the struct itself is immutable is that since both the timestamp and values arrays are getting modified, any copy of the TimeSeries will get modified the same way, though one issue is that copies of the struct may be sharing timestamps (but this would cause problems anyways, specifically with the update method adding new timestamps...)

iblislin commented 4 years ago

If you want I can do a PR. Should we also do filter!, or should we strive to keep everything immutable?

I think the point is getting clearer: the sorted time index is the most important assumption. So if we can get things done without breaking that assumption, it's great. We are seeking a balance point between immutable and mutable object: TimeArray is mutable under some conditions. Then, adding filter! is reasonable for me.

My justification for why filter! would work even if the struct itself is immutable is that since both the timestamp and values arrays are getting modified, any copy of the TimeSeries will get modified the same way, though one issue is that copies of the struct may be sharing timestamps

We can review all the functions, and discuss about the sharing pointer issues, since now we are moving toward the concept "TimeArray is mutable under some conditions".

imbrem commented 4 years ago

If you want I can do a PR. Should we also do filter!, or should we strive to keep everything immutable?

I think the point is getting clearer: the sorted time index is the most important assumption. So if we can get things done without breaking that assumption, it's great. We are seeking a balance point between immutable and mutable object: TimeArray is mutable under some conditions. Then, adding filter! is reasonable for me.

My justification for why filter! would work even if the struct itself is immutable is that since both the timestamp and values arrays are getting modified, any copy of the TimeSeries will get modified the same way, though one issue is that copies of the struct may be sharing timestamps

We can review all the functions, and discuss about the sharing pointer issues, since now we are moving toward the concept "TimeArray is mutable under some conditions".

Question: does the values field accessor copy the values in the TimeArray? If it doesn't, that's bad because we can stick stuff on the end. On the other hand, it would be nice to be able to modify without sticking stuff at the end (since there are no corresponding time stamps). I think returning a SubArray would do the trick?

iblislin commented 4 years ago

Question: does the values field accessor copy the values in the TimeArray?

No, it doesn't copy. If user want to copy it, one should invoke getindex. (like the experience in Julia Array. and we can provide more info in doc and tell users which function copies) I will vote for modifiable values.