Skip to content

feat(resampling): support setting custom dates for the start and end of extrapolation #58

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Jan 24, 2025
Merged

feat(resampling): support setting custom dates for the start and end of extrapolation #58

merged 4 commits into from
Jan 24, 2025

Conversation

Benzhaomin
Copy link
Contributor

@Benzhaomin Benzhaomin commented Dec 10, 2024 •
edited
Loading

Default is still to use the first and last known timestamps:

Screenshot 2024-12-10 at 14 39 22

Custom dates can be set for either:

Screenshot 2024-12-10 at 14 40 17

dss-plugin-timeseries-preparation-2.1.0.zip

@Benzhaomin Benzhaomin added the enhancement New feature or request label Dec 10, 2024
@Benzhaomin Benzhaomin requested a review from tmwly December 10, 2024 13:46
@Benzhaomin Benzhaomin self-assigned this Dec 10, 2024
tmwly
tmwly reviewed Dec 19, 2024
View reviewed changes
Copy link
Contributor

@tmwly tmwly left a comment

Choose a reason for hiding this comment

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

lgtm, tested and working - just one comment about release notes

@nicolasdalsass
Copy link
Contributor

Let's go ahead here ?

@Benzhaomin Benzhaomin requested a review from tmwly January 10, 2025 15:41
tmwly
tmwly reviewed Jan 20, 2025
View reviewed changes
Copy link
Contributor

@tmwly tmwly left a comment

Choose a reason for hiding this comment

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

Overall lgtm

Weirdly, opening a resampling recipe now always dirties the save, which didn't seem to be the case before

Also, maybe we should have a quick sentence explaining what we do in _can_customize_resampling_dates for anyone in a +-13 tz ?

@Benzhaomin
fix(resampling): move datetime to date to param parsing and switch to…
7abde1a 
… favoring UTC+13 (NZDT) over UTC-11 (American Samoa), also add a warning about that behavior in the recipe logs
@Benzhaomin
Copy link
Contributor Author

Benzhaomin commented Jan 22, 2025 •
edited
Loading

Overall lgtm

Weirdly, opening a resampling recipe now always dirties the save, which didn't seem to be the case before

Logged internally as this is common to all plugins with a date input

Also, maybe we should have a quick sentence explaining what we do in _can_customize_resampling_dates for anyone in a +-13 tz ?

We now favour UTC+13 and only have unexpected behaviour in UTC-11 (American Samoa). Code is well commented and a warning is added in the logs in that case.

@Benzhaomin Benzhaomin requested a review from tmwly January 24, 2025 07:24
tmwly
tmwly approved these changes Jan 24, 2025
View reviewed changes
Copy link
Contributor

@tmwly tmwly left a comment

Choose a reason for hiding this comment

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

LGTM! nice work on the timestamp stuff

think we need to merge the changelog but otherwise all clear

@Benzhaomin Benzhaomin merged commit 4a3ef72 into master Jan 24, 2025
@Benzhaomin Benzhaomin deleted the feat/v210-resampling-custom-extrapolation-dates branch January 24, 2025 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tmwly tmwly tmwly approved these changes

Assignees

@Benzhaomin Benzhaomin

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Benzhaomin @nicolasdalsass @tmwly