Matrix: Extract validation from scorer #1883
Conversation
npm Snapshot: PublishedGood news!! We've packaged up the latest commit from this PR (6109412) and published it to npm. You Example: yarn add @khanacademy/perseus@PR1883If you are working in Khan Academy's webapp, you can run: ./dev/tools/bump_perseus_version.sh -t PR1883 |
|
Size Change: +175 B (+0.01%) Total Size: 1.29 MB
ℹ️ View Unchanged
|
GeraldRequired Reviewers
Don't want to be involved in this pull request? Comment |
jeremywiebe
changed the title
| * | ||
| * @see `scoreMatrix()` for more details. | ||
| */ | ||
| function validateMatrix( |
There was a problem hiding this comment.
Question: Did you decide just to leave all the createValidator logic in the scoring function? Do you think this is something we might want to change in the future? Looks like some of it is for scoring, but some is for validation.
There was a problem hiding this comment.
It's all left in the scoring function because there is no logic that can be moved into the validator that doesn't require the scoring data.
There was a problem hiding this comment.
It kind of looked like the hasEmptyCell check could possibly be moved out, but then you'd duplicate a lot of that.
There was a problem hiding this comment.
Nice catch. I need the solution size for part of these check but the hasEmptyCell check can definitely be raised up!
|
I'm confused between this and #1881 |
| }; | ||
| } & PerseusMatrixValidationData; | ||
|
|
||
| export type PerseusMatrixValidationData = Empty; |
There was a problem hiding this comment.
I mentioned this in the other PRs, but just to be extra annoying: I'm not sure we really need a bunch of aliases for Empty.
There was a problem hiding this comment.
I'll remove it (and others that were Empty). I've added a doc comment to the top of this file in another PR.
jeremywiebe
force-pushed
the
jer/validate-matrix
branch
from
dea057c
to
718fc62
Compare
Summary:
This PR extracts validation from the
matrix's scorer function. In reality, it's an empty function, but it follows our conventions for having the scorer call a validator first. I've created the standard tests to ensure that scorer calls the validator.Issue: LEMS-2602
Test plan:
yarn testyarn typecheck