Skip to content

[Proposal] Add range argument to .valid_allin#98

Merged
Dooor merged 1 commit intokufu:masterfrom
Dooor:improve-valid_allin
Aug 16, 2022
Merged

[Proposal] Add range argument to .valid_allin#98
Dooor merged 1 commit intokufu:masterfrom
Dooor:improve-valid_allin

Conversation

@Dooor
Copy link
Copy Markdown
Contributor

@Dooor Dooor commented Aug 8, 2022

Summary

Proposal

when Range with including end (e.g. "2020/02/01".."2020/05/01") is passed, it will be same as previous behavior

Code:  valid_allin(02/01..05/01) / valid_allin(from: 02/01, to: 05/01)
Query: from <= valid_from AND valid_to <= to

01/01         02/01         03/01         04/01         05/01         06/01
  |-------------|*************|*************|*************|-------------|
                ↑                                         ↑
               from(02/01)                                to(05/01)

when Range with excluding end (e.g. "2020/02/01"..."2020/05/01") is passed, it will raise error

Code:  valid_allin(02/01...05/01)

=> RuntimeError: Range with excluding end is not supported

@Dooor Dooor marked this pull request as ready for review August 8, 2022 06:08
@auto-assign auto-assign bot requested review from 16bitidol and k-myst August 8, 2022 06:08
@Dooor Dooor requested a review from osyo-manga August 8, 2022 06:08
Copy link
Copy Markdown
Collaborator

@osyo-manga osyo-manga left a comment

Choose a reason for hiding this comment

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

Great! Thanks :)

@Dooor Dooor requested a review from t-osdy August 9, 2022 01:23
Copy link
Copy Markdown

@16bitidol 16bitidol left a comment

Choose a reason for hiding this comment

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

👍

@Dooor Dooor merged commit cd0cc56 into kufu:master Aug 16, 2022
@Dooor Dooor deleted the improve-valid_allin branch August 16, 2022 01:16
@mkmn mkmn mentioned this pull request Sep 16, 2022
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

Successfully merging this pull request may close these issues.

3 participants