Enhance ResamplerConfig with closed and label options and add comprehensive tests by malteschaaf · Pull Request #1344 · frequenz-floss/frequenz-sdk-python

malteschaaf · 2026-01-14T09:00:34Z

This PR introduces the WindowSide enum and two new configuration options—closed and label—to the ResamplerConfig class, providing a clearer and more flexible way to define the behavior of the resampling process. Comprehensive tests have been added to ensure correctness.

Changes:

Introduce WindowSide enum
- Added the WindowSide enum with values LEFT and RIGHT:
  - LEFT: Represents the start of a resampling window.
  - RIGHT: Represents the end of a resampling window.
- Exposed WindowSide in the __init__.py of the timeseries module for public use.
Add closed option to ResamplerConfig
- Introduced the closed parameter with options WindowSide.RIGHT (default) and WindowSide.LEFT.
- Updated resampling logic to respect the closed configuration:
  - RIGHT: Includes samples at the end of the window, excludes those at the start.
  - LEFT: Includes samples at the start of the window, excludes those at the end.
- Updated documentation to explain the behavior of the closed parameter.
Add label option to ResamplerConfig
- Introduced the label parameter with options WindowSide.RIGHT (default) and WindowSide.LEFT.
- Updated resampling logic to respect the label configuration:
  - RIGHT: Timestamp of the resampled data corresponds to the end of the resampling window.
  - LEFT: Timestamp of the resampled data corresponds to the start of the resampling window.
- Adjusted logic for setting sample_time based on the label configuration.
- Updated documentation accordingly.
Add tests for closed and label
- Verified closed behavior with both WindowSide.RIGHT and WindowSide.LEFT, including additional sample points.
- Verified label behavior to ensure timestamps of resampled data correspond correctly to the start or end of the window.
- Added assertions to confirm correct resampling outputs and updates to source properties and buffer lengths.

src/frequenz/sdk/timeseries/_resampling/_config.py

cwasicki · 2026-01-14T17:24:11Z

src/frequenz/sdk/timeseries/_resampling/_resampler.py

        )
        minimum_relevant_timestamp = timestamp - period * conf.max_data_age_in_periods

-        min_index = bisect(


I think the behavior how to resample, i.e. left or right open and the labeling should be config parameters.

I think we should make left or right opened configurable with the corresponding label, such as:
right open: [t,t+1) -> labeled as t
left open: (t-1,t] -> labeled as t (the current behavior)
Or do you want to allow the user to additionally do something like:
right open, label in the end: [t,t+1) -> labeled as t+1
left open, label in the beginning:(t-1,t] -> labeled as t-1
Because I think the later could lead to a lot of confusion (not sure if its ever needed)

I think the latter is also reasonable options (see e.g. https://pandas.pydata.org/docs/reference/api/pandas.DataFrame.resample.html), but don't see a strong reason to implement this now if not needed. If it's well-documented, the users can also adjust the timestamps trivially. So your proposal sounds good to me.

Yeah, whatever we do, we should probably be much more explicit of how output samples are calculated and structured.

llucax

In general LGTM, I think this would need quite some testing, and we probably want to introduce this using a config option to change the behavior, specially if configurability is something we want to allow in the future (if we don't see having this level of configurability useful, we can hide it in a feature flag using a env var like we did with the new wall clock timer). As we experimented recently with the changes in the component graph, subtle changes in behavior can go easily unnoticed and cause issues.

src/frequenz/sdk/timeseries/_resampling/_config.py

llucax · 2026-02-02T16:07:34Z

src/frequenz/sdk/timeseries/_resampling/_resampler.py

        )
        minimum_relevant_timestamp = timestamp - period * conf.max_data_age_in_periods

-        min_index = bisect(


Yeah, whatever we do, we should probably be much more explicit of how output samples are calculated and structured.

src/frequenz/sdk/timeseries/_resampling/_config.py

malteschaaf · 2026-02-03T16:04:41Z

Ready for Review 🧐

Changes implemented:

Removed modifications to the max_data_age_in_periods default value
Made the openness of the resampling window configurable via the closed parameter:
- "right": Includes the end of the window, excludes the start. (Default: same behavior as before)
- "left": Includes the start of the window, excludes the end.
Added configurability for timestamp labeling with the label parameter:
- "start": Resampled timestamp represents the start of the resampling window.
- "end": Resampled timestamp represents the end of the resampling window. (Default: same behavior as before)
Added comprehensive tests to verify the behavior of the closed and label parameters.

src/frequenz/sdk/timeseries/_resampling/_config.py

malteschaaf · 2026-02-12T12:31:05Z

Ready for Review 🧐

Changes implemented:

Changed label from Literal["start","end"] to Literal["left","right"]

cwasicki

LGTM

src/frequenz/sdk/timeseries/_resampling/_config.py

malteschaaf · 2026-02-12T18:27:49Z

Ready for Review again

Changes:

Intorduced WindowSide enum to represent the sides of a resampling window and used that for the configuration of the closed and label parameters.
Updated the documentation for the closed and label config parameters, removing mapping left and right to "start" and "end"

daniel-zullo-frequenz

LGTM. I'll leave to Luca to review it and approve it

llucax · 2026-03-03T10:34:15Z

Sorry for the delay, will have a look now.

llucax

LGTM, a few very nitpick comments, and one that is moderately important to me (using match instead of if). I know it might look irrelevant, but it looks to me like one of these "64k is enough for everyone" situation, where it seems impossible that anyone could add a new enum member... until someone does.

src/frequenz/sdk/timeseries/_resampling/_config.py

src/frequenz/sdk/timeseries/_resampling/_resampler.py

RELEASE_NOTES.md

- Introduced the `WindowSide` enum with values `LEFT` and `RIGHT`: - `LEFT`: Represents the start of the resampling window. - `RIGHT`: Represents the end of the resampling window. - Exposed the `WindowSide` enum in the `__init__.py` of the timeseries module for public use. Signed-off-by: Malte Schaaf <malte.schaaf@frequenz.com>

…havior - Introduced the `closed` parameter in ResamplerConfig with options `WindowSide.RIGHT` (default) and `WindowSide.LEFT`. - Updated the resampling logic to respect the `closed` configuration: - `RIGHT`: Includes samples at the end of the window, excludes those at the start. - `LEFT`: Includes samples at the start of the window, excludes those at the end. - Adjusted documentation to reflect the new `closed` parameter and its behavior. Signed-off-by: Malte Schaaf <malte.schaaf@frequenz.com>

… resampled data - Introduced the `label` parameter in ResamplerConfig with options `WindowSide.RIGHT` (default) and `WindowSide.LEFT`. - Updated the resampling logic to respect the `label` configuration: - `RIGHT`: The timestamp of the resampled data corresponds to the end of the resampling window. - `LEFT`: The timestamp of the resampled data corresponds to the start of the resampling window. - Adjusted the logic for setting `sample_time` to use the `label` configuration. - Updated documentation to reflect the new `label` parameter and its behavior. Signed-off-by: Malte Schaaf <malte.schaaf@frequenz.com>

- Enhanced the `test_resampler_closed_option` to include additional samples at 2.5, 3, and 4 seconds. - Verified the behavior of the `closed` option (`WindowSide.RIGHT` and `WindowSide.LEFT`) with the extended timeline. - Added assertions to ensure correct resampling function calls and sink outputs for the new samples. - Confirmed that source properties and buffer length are updated correctly after processing the additional samples. Signed-off-by: Malte Schaaf <malte.schaaf@frequenz.com>

…havior - Introduced `test_resampler_label_option` to validate the `label` configuration in ResamplerConfig. - Tested both `WindowSide.LEFT` and `WindowSide.RIGHT` options to ensure the resampled datas timestamp corresponds to the start or end of the resampling window, respectively. - Verified sink outputs with the expected timestamp and resampled value. Signed-off-by: Malte Schaaf <malte.schaaf@frequenz.com>

Improve the docstring of the `_StreamingHelper`s `resample` method to better describe how the timestamp parameter is used in resampling calculations. The timestamp now clearly represents the upper bound for buffered samples considered in the resampling operation. Signed-off-by: Malte Schaaf <malte.schaaf@frequenz.com>

Signed-off-by: Malte Schaaf <malte.schaaf@frequenz.com>

malteschaaf · 2026-03-04T08:35:15Z

READY FOR REVIEW

Rebased
Changed Release Notes according to comment
match enum instead of if
Updated the docstring of the resample method of the StreamingHelper class to better describe how the timestamp parameter is used in resampling calculations

llucax

🙏

malteschaaf · 2026-03-04T13:08:59Z

Unfortunately I have no rights to merge myself @llucax

github-project-automation bot added this to Python SDK Roadmap Jan 14, 2026

github-project-automation bot moved this to To do in Python SDK Roadmap Jan 14, 2026

github-actions bot added the part:data-pipeline Affects the data pipeline label Jan 14, 2026

malteschaaf requested review from cwasicki and llucax January 14, 2026 09:22

cwasicki reviewed Jan 14, 2026

View reviewed changes

llucax reviewed Feb 2, 2026

View reviewed changes

malteschaaf force-pushed the resampler branch from 83c6774 to 96dbff9 Compare February 3, 2026 10:08

github-actions bot added the part:tests Affects the unit, integration and performance (benchmarks) tests label Feb 3, 2026

malteschaaf changed the title ~~WIP: Adjust Resampling Logic and Defaults~~ Enhance ResamplerConfig with closed and label options and add comprehensive tests Feb 3, 2026

malteschaaf changed the title ~~Enhance ResamplerConfig with closed and label options and add comprehensive tests~~ Enhance ResamplerConfig with closed and label options and add comprehensive tests Feb 3, 2026

github-actions bot added the part:docs Affects the documentation label Feb 3, 2026

malteschaaf requested review from cwasicki and llucax February 3, 2026 16:07

malteschaaf force-pushed the resampler branch from 57865ce to 5bd2919 Compare February 3, 2026 16:13

malteschaaf marked this pull request as ready for review February 3, 2026 16:14

malteschaaf requested a review from a team as a code owner February 3, 2026 16:14

malteschaaf force-pushed the resampler branch from 5bd2919 to 297ec7c Compare February 3, 2026 16:21

cwasicki reviewed Feb 9, 2026

View reviewed changes

src/frequenz/sdk/timeseries/_resampling/_config.py Outdated Show resolved Hide resolved

malteschaaf force-pushed the resampler branch 2 times, most recently from b39f3de to 1231852 Compare February 12, 2026 12:29

malteschaaf requested a review from cwasicki February 12, 2026 12:31

cwasicki reviewed Feb 12, 2026

View reviewed changes

daniel-zullo-frequenz reviewed Feb 12, 2026

View reviewed changes

src/frequenz/sdk/timeseries/_resampling/_config.py Outdated Show resolved Hide resolved

daniel-zullo-frequenz reviewed Feb 12, 2026

View reviewed changes

src/frequenz/sdk/timeseries/_resampling/_config.py Outdated Show resolved Hide resolved

malteschaaf force-pushed the resampler branch from 1231852 to d99614a Compare February 12, 2026 18:17

malteschaaf force-pushed the resampler branch from d99614a to f745efc Compare February 12, 2026 18:43

malteschaaf requested review from cwasicki and daniel-zullo-frequenz February 12, 2026 18:47

daniel-zullo-frequenz reviewed Feb 12, 2026

View reviewed changes

llucax reviewed Mar 3, 2026

View reviewed changes

malteschaaf added 7 commits March 4, 2026 08:58

Update release notes

81a8840

Signed-off-by: Malte Schaaf <malte.schaaf@frequenz.com>

malteschaaf force-pushed the resampler branch from f745efc to 81a8840 Compare March 4, 2026 08:30

malteschaaf requested a review from llucax March 4, 2026 08:35

llucax approved these changes Mar 4, 2026

View reviewed changes

github-project-automation bot moved this from To do to Review approved in Python SDK Roadmap Mar 4, 2026

Conversation

malteschaaf commented Jan 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes:

Uh oh!

Uh oh!

cwasicki Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

malteschaaf Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

cwasicki Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

llucax Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

llucax left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

llucax Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

malteschaaf commented Feb 3, 2026

Uh oh!

Uh oh!

malteschaaf commented Feb 12, 2026

Uh oh!

cwasicki left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

malteschaaf commented Feb 12, 2026

Uh oh!

daniel-zullo-frequenz left a comment

Choose a reason for hiding this comment

Uh oh!

llucax commented Mar 3, 2026

Uh oh!

llucax left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

malteschaaf commented Mar 4, 2026

Uh oh!

llucax left a comment

Choose a reason for hiding this comment

Uh oh!

malteschaaf commented Mar 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

malteschaaf commented Jan 14, 2026 •

edited

Loading

llucax left a comment •

edited

Loading