-
Notifications
You must be signed in to change notification settings - Fork 2
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: add input validation for non-private methods of approximation.py #260
Conversation
@tp832944 first module covered for an initial check of the approach. I suggest we don't verify inputs of private functions, as these are not intended to be accessed by users. We need to clarify exactly what is an acceptable input if something is just 'array like' and how to check this. Also, we should verify what to do with JIT decorated functions - if they check only on first call, then we aren't validating on future calls when compiled. I suggest we don't input validate in these cases. |
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.
Your validation function idea is good @pc532627. I've suggested several changes to keep it more general.
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.
Thank you for your changes @pc532627. Here are a few more please.
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.
All good @pc532627. Now for someone else to extend it to the rest of the codebase (new PR).
Refs: 251
PR Type
Description
Input validation added for approximation.py as a check before proceeding with other modules.
How Has This Been Tested?
Test A: test_kernel_mean_approximator_creation_invalid_types
Test B: test_random_approximator_creation_invalid_types
Test C: test_annchor_approximator_creation_invalid_types
Test D: test_nystrom_approximator_creation_invalid_types
Checklist before requesting a review