Move NumericInput and InputNumber scoring logic to perseus-score
#2105
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
handeyeco
changed the title
perseus-score
npm Snapshot: PublishedGood news!! We've packaged up the latest commit from this PR (2e64b66) and published it to npm. You Example: yarn add @khanacademy/perseus@PR2105If you are working in Khan Academy's webapp, you can run: ./dev/tools/bump_perseus_version.sh -t PR2105 |
|
Size Change: -97 B (-0.01%) Total Size: 1.47 MB
ℹ️ View Unchanged
|
jeremywiebe
approved these changes
Jan 16, 2025
| @@ -271,11 +270,7 @@ describe("invalid", function () { | |||
| }); | |||
|
|
|||
| it("should handle invalid answers with no error callback", function () { | |||
There was a problem hiding this comment.
Can we move this test to perseus-score?
| @@ -140,7 +140,7 @@ function walkTex( | |||
| * expression will have its innermost fractions stubbed out with \fracs | |||
| * (as opposed to \dfracs). All other content will remain untouched. | |||
| */ | |||
| function modifyTex(tex: string): string { | |||
| export function modifyTex(tex: string): string { | |||
There was a problem hiding this comment.
It looks like the primary reason we need to export this is so that we can attach it to the numeric-input's WidgetExports.
I'd really love it if when we're all done that the individual widget scoring functions are unexported. That will help to keep the surface area of perseus-score tighter and avoid applications from binding to it and making it harder to change if we need to.
There was a problem hiding this comment.
I'd really love it if when we're all done that the individual widget scoring functions are unexported.
I think that's totally doable. It would just mean that the widget type-to-scorer map would be internal to perseus-score and not something set by the consumer.
| @@ -9,41 +10,6 @@ import type {PerseusInputNumberWidgetOptions} from "@khanacademy/perseus-core"; | |||
|
|
|||
| const {InfoTip} = components; | |||
|
|
|||
| const answerTypes = { | |||
There was a problem hiding this comment.
Thanks for catching this duplication and cleaning it up!
* move radio scorer/validator * lint errorToString better * comment my unique type * respond to Jeremy's feedback * Move scoring logic: CSProgram, Iframe, and Dropdown (#2111) * cs-program * move dropdown * move iframe * Move scoring logic: Table, NumberLine, Matcher (#2112) * move table * number-line * move matcher * Move scorers: Plotter and Sorter (#2113) * plotter * sorter * Move scorer: Orderer (#2114) * orderer * Move scorerer: LabelImage (#2115) * move label-image * rename labelImageScoreMarker * Move scoring logic: Matrix (#2116) * move matrix * Move scoring logic: Expression (#2118) * move expression scorer * respond to Ben's feedback * merge Grapher move * fix conflict again * Move scorer: Grapher (#2119) * move matrix * move expression scorer * move grapher scorer * respond to Ben's feedback * Move scorer: Interactive Graph (#2120) * STOPSHIP some type errors still * add back duplicate declarations * add back Line duplicate * all tests passing * Revert changes to underscore's isEqual (#2125) ## Summary: [Original comment](#2113 (comment)) I made a separate PR because I made this mistake in a couple of PRs so I thought I'd knock them out all at once. Issue: LEMS-2737 ## Test plan: Author: handeyeco Reviewers: jeremywiebe, benchristel Required Reviewers: Approved By: jeremywiebe Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ❌ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ⏹️ [cancelled] Publish npm snapshot (ubuntu-latest, 20.x), ⏹️ [cancelled] Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ⏹️ [cancelled] Check builds for changes in size (ubuntu-latest, 20.x), ⏹️ [cancelled] Cypress (ubuntu-latest, 20.x), ⏹️ [cancelled] Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ⏹️ [cancelled] Publish npm snapshot (ubuntu-latest, 20.x), ⏹️ [cancelled] Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ⏹️ [cancelled] Check builds for changes in size (ubuntu-latest, 20.x), ⏹️ [cancelled] Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ⏹️ [cancelled] Cypress (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x) Pull Request URL: #2125 * respond to Jeremy's feedback
handeyeco
added a commit
that referenced
this pull request
Jan 17, 2025
* move categorizer scoring logic * Move NumericInput and InputNumber scoring logic to `perseus-score` (#2105) * move numeric-input and input-number * respond to Jeremy's feedback * Move Radio scorer/validator (#2106) * move radio scorer/validator * lint errorToString better * comment my unique type * respond to Jeremy's feedback * Move scoring logic: CSProgram, Iframe, and Dropdown (#2111) * cs-program * move dropdown * move iframe * Move scoring logic: Table, NumberLine, Matcher (#2112) * move table * number-line * move matcher * Move scorers: Plotter and Sorter (#2113) * plotter * sorter * Move scorer: Orderer (#2114) * orderer * Move scorerer: LabelImage (#2115) * move label-image * rename labelImageScoreMarker * Move scoring logic: Matrix (#2116) * move matrix * Move scoring logic: Expression (#2118) * move expression scorer * respond to Ben's feedback * merge Grapher move * fix conflict again * Move scorer: Grapher (#2119) * move matrix * move expression scorer * move grapher scorer * respond to Ben's feedback * Move scorer: Interactive Graph (#2120) * STOPSHIP some type errors still * add back duplicate declarations * add back Line duplicate * all tests passing * Revert changes to underscore's isEqual (#2125) ## Summary: [Original comment](#2113 (comment)) I made a separate PR because I made this mistake in a couple of PRs so I thought I'd knock them out all at once. Issue: LEMS-2737 ## Test plan: Author: handeyeco Reviewers: jeremywiebe, benchristel Required Reviewers: Approved By: jeremywiebe Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ❌ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ⏹️ [cancelled] Publish npm snapshot (ubuntu-latest, 20.x), ⏹️ [cancelled] Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ⏹️ [cancelled] Check builds for changes in size (ubuntu-latest, 20.x), ⏹️ [cancelled] Cypress (ubuntu-latest, 20.x), ⏹️ [cancelled] Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ⏹️ [cancelled] Publish npm snapshot (ubuntu-latest, 20.x), ⏹️ [cancelled] Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ⏹️ [cancelled] Check builds for changes in size (ubuntu-latest, 20.x), ⏹️ [cancelled] Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ⏹️ [cancelled] Cypress (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x) Pull Request URL: #2125 * respond to Jeremy's feedback
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
I ended up moving them at the same time because of their shared dependency on TexWrangler.
perseus-scoreanswerTypesIssue: LEMS-2737
Test plan:
Nothing should change, just moving code