Change zero to nan for lack of data #12
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Flora <[email protected]>
flora-hofmann-frequenz
force-pushed
the
zero_change
branch
from
75bd431 to
c67fa1d
Compare
cwasicki
approved these changes
Oct 1, 2024
There was a problem hiding this comment.
LGTM.
I realized some unrelated things though:
- We rely on the data being sorted in the sum. This is currently usually the case but it's not guaranteed by the service.
- If we have missing data this can be really off, depending on the cause of the missing data. We currently effectively treat NaNs as zeroes and forward-fill gaps in case we use raw data. This is inconsistent and I think we should just forbid resolution of
Nones (although this wouldn't really fill gaps in the current version of the service, but in future only). - I think the start time of the
CumulativeEnergyshould be the timestamp of the first sample. The end time I think is fine if we always assume resampled data, otherwise we would need to adjust it too.
I have opened a new issue for these points |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As identified by Noah if there is no data we should get
naninstead of0.0