-
Notifications
You must be signed in to change notification settings - Fork 317
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
Allow GreaterThan constraint to work with timestamp #597
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.
LGTM!
Just a comment about adding blank lines.
@@ -320,10 +320,14 @@ def _validate_inputs(cls, low, high, scalar, drop): | |||
cls._validate_drop(scalar, drop) | |||
high = cls._validate_scalar(scalar_column=low, column_names=high, scalar=scalar) | |||
constraint_columns = tuple(high) | |||
if isinstance(low, pd.Timestamp): | |||
low = low.to_datetime64() |
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.
Minor detail, can we add a blank line below this one?
elif scalar == 'high': | ||
cls._validate_drop(scalar, drop) | ||
low = cls._validate_scalar(scalar_column=high, column_names=low, scalar=scalar) | ||
constraint_columns = tuple(low) | ||
if isinstance(high, pd.Timestamp): | ||
high = high.to_datetime64() |
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.
Same as above.
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.
Currently, the GreaterThan constraint crashes when passed a pd.Timestamp value. Convert the timestamp to a datetime so that it is able to be compared.
Resolves #596