Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dropinf and other filter functions #436

Open
iblislin opened this issue Oct 21, 2019 · 4 comments
Open

dropinf and other filter functions #436

iblislin opened this issue Oct 21, 2019 · 4 comments

Comments

@iblislin
Copy link
Collaborator

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

@imbrem
Copy link

imbrem commented Jun 4, 2020

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
Copy link
Collaborator Author

iblislin commented Jun 6, 2020

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
Copy link

imbrem commented Jun 7, 2020

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
Copy link
Collaborator Author

iblislin commented Jun 8, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants