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

Cli resample sensor #360

Merged
merged 14 commits into from Feb 12, 2022
Merged

Cli resample sensor #360

merged 14 commits into from Feb 12, 2022

Conversation

Flix6x
Copy link
Contributor

@Flix6x Flix6x commented Feb 8, 2022

This PR adds a CLI command to resample sensor data to a different resolution.

Requires SeitaBV/timely-beliefs#99.

Signed-off-by: F.N. Claessen <felix@seita.nl>
…accordingly

Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
@Flix6x Flix6x added the CLI label Feb 8, 2022
@Flix6x Flix6x requested a review from nhoening February 8, 2022 15:54
@Flix6x Flix6x self-assigned this Feb 8, 2022
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Copy link
Contributor

@nhoening nhoening left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

We now know how to test CLI tasks, so I wonder if we can add a small test. Is there some sample series in some conftest we can resample?

Also: This resamples all data of the sensor. Which is fine. But I have a question about this in practice: I can imagine that new data with a different resolution already started coming in.

  • Will our APIs catch that problem at the moment?
  • Will this task still be able to resample in the case that data with a different resolution made it in?

# Conflicts:
#	documentation/changelog.rst
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
@Flix6x
Copy link
Contributor Author

Flix6x commented Feb 12, 2022

Great suggestion! The CLI command now let's you pass a time window.

@Flix6x Flix6x requested a review from nhoening February 12, 2022 20:23
@Flix6x Flix6x merged commit 17acc1c into main Feb 12, 2022
@Flix6x Flix6x deleted the cli-resample-sensor branch February 12, 2022 22:56
@Flix6x Flix6x added this to the 0.9.0 milestone Mar 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants