-
Notifications
You must be signed in to change notification settings - Fork 39
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
Feat: threshold checker interval string #652
Feat: threshold checker interval string #652
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some comments for consideration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only one issue outstanding.
16b9ffa
to
3fa2f48
Compare
3fa2f48
to
24dbbc9
Compare
Set an upper limit to |
Looks good to merge. |
Closes https://github.com/aimakerspace/PeekingDuck-Private/issues/83
Changes:
The interval string must match
^[\[\(]\s*[-+]?(inf|\d*\.?\d+)\s*,\s*[-+]?(inf|\d*\.?\d+)\s*[\]\)]$
:^
: Start of line[\[\(]
: Either[
or(
\s*
: Any whitespace characters between 0 and unlimited times[-+]?
:-
or+
between 0 and 1 time(inf|\d*\.?\d+)
: Eitherinf
or a decimal number where having a digit before the period is optional, e.g.,.9
. Ref comment by Chandranshu\s*
: Whitespace,
: Separator\s*
: Whitespace[-+]?(inf|\d*\.?\d+)
: Optional-
or+
with eitherinf
or a decimal number\s*
: Whitespace[\]\)]
: Either]
or)
$
: End of lineSince we are trying to use a mathematical representation of the interval, I have opted to use
-inf
and+inf
for correctness.