Closed andrewelamb closed 4 years ago
Looks great! I'm wondering - validate_submission
appears to be an example, rather than the actual function a user would implement, right?
I'm thinking a model where you could pass in a list of validation functions to validate_submission
might make sense. It would be a pretty simple wrapper function, but it might help the user figure out how to actually use the validation helper functions, or write their own?
Nice @andrewelamb ! Would these validation functions be called in a scoring function? Also, it could be worth it to have a function that checks the two index columns. (e.g. Prediction column A must have all values in truth column A and vice versa)
@thomasyu888 In my mind the ones I have so far are checks that should be done in most challenges irregardless of the scoring functions, so would be called first before any scoring functions were called.
I have code that does the missing value checking, it just wasn't pushed yet :)
@allaway I'm going to make validate_submission available, but users can write their own function that make uses of the lower level functions. Some of these need to be done in order to make sure there are no errors(like checking that columns exist before doing a join to check for missing predictions).
@andrewelamb thanks for the clarification, that makes sense!
@thomasyu888 @allaway I've added some more code. Please let me know what you think of the general direction, if there any any checks I should add.
@thomasyu888 @allaway This ready as a real pull request, please comment, or if you have no comments go ahead and merge
apologies for the post-merge review, but lgtm!
On Tue, May 19, 2020 at 10:20 AM andrewelamb notifications@github.com wrote:
Merged #18 https://github.com/Sage-Bionetworks/challengescoring/pull/18 into develop.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Sage-Bionetworks/challengescoring/pull/18#event-3352928754, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE3WNSFHTJSIHBLLDS7RLQ3RSK5XRANCNFSM4M3OCVJQ .
No need to merge this pull request, I just wanted to show what my take on the validation functions look like.