-
Notifications
You must be signed in to change notification settings - Fork 8
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
Fix & improve .window_size validation #535
Conversation
* Mention what's acceptable for yearmonth time_type * Mention Inf in validation errors as acceptable iff it's actually acceptable. * Reject any other strange "int" classes that pass test_int. (It rejects Date and POSIXt, but perhaps there are others.) * Refactor to use helper function test_sensible_int instead of test_int (as latter accepts difftimes and makes logic look confusing).
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.
Ah, I'm seeing a bunch of minor improvements to the slide args I didn't move over to epi_slide_opt, I can add a commit in a little. Thanks for noticing!
|
Yea I didn't port the validation from epi_slide over epi_slide_opt, but I'll do that now. |
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.
Ok looks better now. Should be good to go.
Checklist
Please:
PR).
brookslogan, nmdefries.
DESCRIPTION
. Always incrementthe patch version number (the third number), unless you are making a
release PR from dev to main, in which case increment the minor version
number (the second number).
(backwards-incompatible changes to the documented interface) are noted.
Collect the changes under the next release number (e.g. if you are on
1.7.2, then write your changes under the 1.8 heading).
process.
Change explanations for reviewer
and POSIXt, but perhaps there are others.)
latter accepts difftimes and makes logic look confusing).-
Magic GitHub syntax to mark associated Issue(s) as resolved when this is merged into the default branch