Skip to content
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

Rename usages of rubric to scoringData #2006

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
1033ce5
Rename rubric in dropdown
Myranae Dec 13, 2024
4502233
Rename rubric in expression
Myranae Dec 13, 2024
cde74a0
Rename rubric in group
Myranae Dec 13, 2024
3259ca1
Rename rubric in graded group
Myranae Dec 13, 2024
ebbae69
Rename rubric in graded group set
Myranae Dec 13, 2024
9b5280e
Rename rubric in grapher
Myranae Dec 13, 2024
f2fe73d
Rename rubric in input number
Myranae Dec 13, 2024
d6459e5
Rename rubric in interactive graph
Myranae Dec 13, 2024
2bf2e36
Rename rubric in matcher
Myranae Dec 13, 2024
cfcf63f
Rename rubric in matrix
Myranae Dec 13, 2024
824294b
Rename rubric in numeric input
Myranae Dec 13, 2024
d35f96b
Rename rubric in orderer
Myranae Dec 13, 2024
e2ec414
Rename rubric in radio
Myranae Dec 13, 2024
82aef45
Rename rubric in sorter
Myranae Dec 13, 2024
5159f2a
Rename rubric in table
Myranae Dec 13, 2024
10476c0
Rename Rubric type to ScoringData
Myranae Dec 13, 2024
89a7a34
Rename state to userInput for scoreMatcher
Myranae Dec 13, 2024
74fc307
Rename getOneCorrectAnswerFromRubric
Myranae Dec 13, 2024
d086146
Add changeset
Myranae Dec 13, 2024
356594d
Rename state to userInput for scoreCSProgram
Myranae Dec 13, 2024
9afbacb
Merge branch 'feature/client-validation' into tb/LEMS-2657/rename-rub…
Myranae Dec 17, 2024
c9de205
Rename type in test
Myranae Dec 17, 2024
c45792b
Merge branch 'feature/client-validation' into tb/LEMS-2657/rename-rub…
Myranae Dec 18, 2024
938d6a7
Rename some missed rubric usages
Myranae Dec 18, 2024
ce47cf6
Some more missed renaming
Myranae Dec 18, 2024
6f1c890
Update comment to align with interface
Myranae Dec 18, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/perseus/src/renderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,7 @@ class Renderer
const apiOptions = this.getApiOptions();
const widgetProps = this.state.widgetProps[widgetId] || {};

// The widget needs access to its "rubric" at all times when in review
// The widget needs access to its "scoring data" at all times when in review
// mode (which is really just part of its widget info).
const widgetInfo = this.state.widgetInfo[widgetId];
const reviewModeRubric =
Expand Down
2 changes: 1 addition & 1 deletion packages/perseus/src/validation.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ export interface ValidationDataTypes {
categorizer: PerseusCategorizerValidationData;
// "cs-program": PerseusCSProgramValidationData;
// definition: PerseusDefinitionValidationData;
// dropdown: PerseusDropdownRubric;
// dropdown: PerseusDropdownValidationData;
// explanation: PerseusExplanationValidationData;
// expression: PerseusExpressionValidationData;
// grapher: PerseusGrapherValidationData;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ function scoreExpression(
Log.error(
"Unable to parse solution answer for expression",
Errors.InvalidInput,
{loggedMetadata: {rubric: JSON.stringify(scoringData)}},
{loggedMetadata: {scoringData: JSON.stringify(scoringData)}},
);
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type {PerseusExpressionUserInput} from "../../validation.types";
/**
* Checks user input from the expression widget to see if it is scorable.
*
* Note: Most of the expression widget's validation requires the Rubric because
* Note: Most of the expression widget's validation requires the ScoringData because
* of its use of KhanAnswerTypes as a core part of scoring.
*
* @see `scoreExpression()` for more details.
Expand Down
12 changes: 7 additions & 5 deletions packages/perseus/src/widgets/input-number/input-number.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -288,13 +288,15 @@ export default {
// @ts-expect-error: Type 'UserInput' is not assignable to type 'PerseusInputNumberUserInput'.
scorer: scoreInputNumber,

getOneCorrectAnswerFromScoringData(rubric: any): string | null | undefined {
if (rubric.value == null) {
getOneCorrectAnswerFromScoringData(
scoringData: any,
): string | null | undefined {
if (scoringData.value == null) {
return;
}
let answerString = String(rubric.value);
if (rubric.inexact && rubric.maxError) {
answerString += " \u00B1 " + rubric.maxError;
let answerString = String(scoringData.value);
if (scoringData.inexact && scoringData.maxError) {
answerString += " \u00B1 " + scoringData.maxError;
}
return answerString;
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import type {PerseusInputNumberScoringData} from "../../validation.types";

describe("scoreInputNumber", () => {
it("scores correct answer correctly", () => {
const rubric: PerseusInputNumberScoringData = {
const scoringData: PerseusInputNumberScoringData = {
maxError: 0.1,
inexact: false,
value: 1,
Expand All @@ -18,13 +18,13 @@ describe("scoreInputNumber", () => {
currentValue: "1",
} as const;

const score = scoreInputNumber(useInput, rubric, mockStrings);
const score = scoreInputNumber(useInput, scoringData, mockStrings);

expect(score).toHaveBeenAnsweredCorrectly();
});

it("scores incorrect answer correctly", () => {
const rubric: PerseusInputNumberScoringData = {
const scoringData: PerseusInputNumberScoringData = {
maxError: 0.1,
inexact: false,
value: 1,
Expand All @@ -36,13 +36,13 @@ describe("scoreInputNumber", () => {
currentValue: "2",
} as const;

const score = scoreInputNumber(useInput, rubric, mockStrings);
const score = scoreInputNumber(useInput, scoringData, mockStrings);

expect(score).toHaveBeenAnsweredIncorrectly();
});

it("shows as invalid with a nonsense answer", () => {
const rubric: PerseusInputNumberScoringData = {
const scoringData: PerseusInputNumberScoringData = {
maxError: 0.1,
inexact: false,
value: 1,
Expand All @@ -54,7 +54,7 @@ describe("scoreInputNumber", () => {
currentValue: "sadasdfas",
} as const;

const score = scoreInputNumber(useInput, rubric, mockStrings);
const score = scoreInputNumber(useInput, scoringData, mockStrings);

expect(score).toHaveInvalidInput(
"We could not understand your answer. Please check your answer for extra text or symbols.",
Expand All @@ -68,7 +68,7 @@ describe("scoreInputNumber", () => {
// important to the test.
// https://khanacademy.atlassian.net/browse/LC-691
it("doesn't default to validating pi", () => {
const rubric: PerseusInputNumberScoringData = {
const scoringData: PerseusInputNumberScoringData = {
maxError: 0.1,
inexact: false,
value: 241.90263432641407,
Expand All @@ -82,7 +82,7 @@ describe("scoreInputNumber", () => {
currentValue: "241.91",
} as const;

const score = scoreInputNumber(userInput, rubric, mockStrings);
const score = scoreInputNumber(userInput, scoringData, mockStrings);

expect(score.message).not.toBe(
"Your answer is close, but yyou may " +
Expand All @@ -95,7 +95,7 @@ describe("scoreInputNumber", () => {
});

it("validates against pi if provided in answerType", () => {
const rubric: PerseusInputNumberScoringData = {
const scoringData: PerseusInputNumberScoringData = {
maxError: 0.1,
inexact: false,
value: 241.90263432641407,
Expand All @@ -107,7 +107,7 @@ describe("scoreInputNumber", () => {
currentValue: "77 pi",
} as const;

const score = scoreInputNumber(userInput, rubric, mockStrings);
const score = scoreInputNumber(userInput, scoringData, mockStrings);

expect(score).toHaveBeenAnsweredCorrectly();
});
Expand Down
16 changes: 8 additions & 8 deletions packages/perseus/src/widgets/input-number/score-input-number.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,24 +47,24 @@ export const answerTypes = {

function scoreInputNumber(
userInput: PerseusInputNumberUserInput,
rubric: PerseusInputNumberScoringData,
scoringData: PerseusInputNumberScoringData,
strings: PerseusStrings,
): PerseusScore {
if (rubric.answerType == null) {
rubric.answerType = "number";
if (scoringData.answerType == null) {
scoringData.answerType = "number";
}

// note(matthewc): this will get immediately parsed again by
// `KhanAnswerTypes.number.convertToPredicate`, but a string is
// expected here
const stringValue = `${rubric.value}`;
const stringValue = `${scoringData.value}`;
const val = KhanAnswerTypes.number.createValidatorFunctional(
stringValue,
{
simplify: rubric.simplify,
inexact: rubric.inexact || undefined,
maxError: rubric.maxError,
forms: answerTypes[rubric.answerType].forms,
simplify: scoringData.simplify,
inexact: scoringData.inexact || undefined,
maxError: scoringData.maxError,
forms: answerTypes[scoringData.answerType].forms,
},
strings,
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ describe("static function getOneCorrectAnswerFromScoringData", () => {
);
});

it("can get one correct answer from a rubric with multiple answers", () => {
it("can get one correct answer from scoring data with multiple answers", () => {
const widget = multipleAnswersWithDecimals.widgets["numeric-input 1"];
const widgetOptions = widget && widget.options;
const answers: ReadonlyArray<any> =
Expand All @@ -180,7 +180,7 @@ describe("static function getOneCorrectAnswerFromScoringData", () => {
expect(singleAnswer).toBe("12.2");
});

it("can get one correct answer from a rubric with one answer", () => {
it("can get one correct answer from scoring data with one answer", () => {
const widget = question1.widgets["numeric-input 1"];
const widgetOptions = widget && widget.options;
const answers: ReadonlyArray<any> =
Expand All @@ -193,7 +193,7 @@ describe("static function getOneCorrectAnswerFromScoringData", () => {
expect(singleAnswer).toBe("1252");
});

it("can not get a correct answer from a rubric with no answer", () => {
it("can not get a correct answer from scoring data with no answer", () => {
const answers: Array<never> = [];
const singleAnswer =
NumericInputWidgetExport.getOneCorrectAnswerFromScoringData?.({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@ export default {
scorer: scoreNumericInput,

// TODO(LEMS-2656): remove TS suppression
// @ts-expect-error: Type 'Rubric' is not assignable to type 'PerseusNumericInputRubric'
// @ts-expect-error: Type 'ScoringData' is not assignable to type 'PerseusNumericInputScoringData'
getOneCorrectAnswerFromScoringData(
scoringData: PerseusNumericInputScoringData,
): string | null | undefined {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ describe("scoreNumericInput", () => {
});

it("is correct when input is empty but answer is 1 and coefficient: true", () => {
const rubric: PerseusNumericInputScoringData = {
const scoringData: PerseusNumericInputScoringData = {
answers: [
{
value: 1,
Expand All @@ -34,7 +34,7 @@ describe("scoreNumericInput", () => {
currentValue: "",
};

const score = scoreNumericInput(userInput, rubric, mockStrings);
const score = scoreNumericInput(userInput, scoringData, mockStrings);

expect(score).toHaveBeenAnsweredCorrectly();
});
Expand Down
24 changes: 12 additions & 12 deletions packages/perseus/src/widgets/radio/__tests__/radio.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -953,7 +953,7 @@ describe("Radio Widget", () => {
/**
* (LEMS-2435) We want to be sure that we're able to score shuffled
* Radio widgets outside of the component which means `getUserInput`
* should return the same order that the rubric provides
* should return the same order that the scoring data provides
*/
it("can be scored correctly when shuffled", async () => {
// Arrange
Expand All @@ -966,8 +966,8 @@ describe("Radio Widget", () => {

const userInput =
renderer.getUserInput()[0] as PerseusRadioUserInput;
const rubric = shuffledQuestion.widgets["radio 1"].options;
const widgetScore = scoreRadio(userInput, rubric, mockStrings);
const scoringData = shuffledQuestion.widgets["radio 1"].options;
const widgetScore = scoreRadio(userInput, scoringData, mockStrings);
const rendererScore = scorePerseusItemTesting(
shuffledQuestion,
renderer.getUserInputMap(),
Expand All @@ -981,7 +981,7 @@ describe("Radio Widget", () => {
/**
* (LEMS-2435) We want to be sure that we're able to score shuffled
* Radio widgets outside of the component which means `getUserInput`
* should return the same order that the rubric provides
* should return the same order that the scoring data provides
*/
it("can be scored incorrectly when shuffled", async () => {
// Arrange
Expand All @@ -994,8 +994,8 @@ describe("Radio Widget", () => {

const userInput =
renderer.getUserInput()[0] as PerseusRadioUserInput;
const rubric = shuffledQuestion.widgets["radio 1"].options;
const widgetScore = scoreRadio(userInput, rubric, mockStrings);
const scoringData = shuffledQuestion.widgets["radio 1"].options;
const widgetScore = scoreRadio(userInput, scoringData, mockStrings);
const rendererScore = scorePerseusItemTesting(
shuffledQuestion,
renderer.getUserInputMap(),
Expand All @@ -1009,7 +1009,7 @@ describe("Radio Widget", () => {
/**
* (LEMS-2435) We want to be sure that we're able to score shuffled
* Radio widgets outside of the component which means `getUserInput`
* should return the same order that the rubric provides
* should return the same order that the scoring data provides
*/
it("can be scored correctly when shuffled with none of the above", async () => {
// Arrange
Expand All @@ -1022,8 +1022,8 @@ describe("Radio Widget", () => {

const userInput =
renderer.getUserInput()[0] as PerseusRadioUserInput;
const rubric = shuffledNoneQuestion.widgets["radio 1"].options;
const widgetScore = scoreRadio(userInput, rubric, mockStrings);
const scoringData = shuffledNoneQuestion.widgets["radio 1"].options;
const widgetScore = scoreRadio(userInput, scoringData, mockStrings);
const rendererScore = scorePerseusItemTesting(
shuffledNoneQuestion,
renderer.getUserInputMap(),
Expand All @@ -1037,7 +1037,7 @@ describe("Radio Widget", () => {
/**
* (LEMS-2435) We want to be sure that we're able to score shuffled
* Radio widgets outside of the component which means `getUserInput`
* should return the same order that the rubric provides
* should return the same order that the scoring data provides
*/
it("can be scored incorrectly when shuffled with none of the above", async () => {
// Arrange
Expand All @@ -1050,8 +1050,8 @@ describe("Radio Widget", () => {

const userInput =
renderer.getUserInput()[0] as PerseusRadioUserInput;
const rubric = shuffledNoneQuestion.widgets["radio 1"].options;
const widgetScore = scoreRadio(userInput, rubric, mockStrings);
const scoringData = shuffledNoneQuestion.widgets["radio 1"].options;
const widgetScore = scoreRadio(userInput, scoringData, mockStrings);
const rendererScore = scorePerseusItemTesting(
shuffledQuestion,
renderer.getUserInputMap(),
Expand Down
6 changes: 3 additions & 3 deletions packages/perseus/src/widgets/radio/radio-component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -268,12 +268,12 @@ class Radio extends React.Component<Props> implements Widget {
*/
showRationalesForCurrentlySelectedChoices: (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My only reservation with these changes are touching some of the things we need to address in the future: showRationalesForCurrentlySelectedChoices, reviewModeRubric, etc. I think it's fine to ship the changes, there's just something in the back of my mind worried that we'll lose some context by renaming things around these APIs before we actually work on them.

I have no justification for this concern, so no action needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@handeyeco I can go through and leave a link to this PR on the relevant tickets and just point out that the naming may have changed and here's where you can see where that happened. I think I might also suggest that if it hasn't been renamed that it get renamed in the course of whatever ticket work happens. Does that sound like a good way to help prevent some loss of context?

I am also 100% okay with putting out another PR reverting any naming changes if we decide after I've landed this that we don't actually want some of these things renamed.

arg1: PerseusRadioWidgetOptions,
) => void = (rubric) => {
) => void = (scoringData) => {
const {choiceStates} = this.props;
if (choiceStates) {
const score = scoreRadio(
this.getUserInput(),
rubric,
scoringData,
this.context.strings,
);
const widgetCorrect =
Expand Down Expand Up @@ -417,7 +417,7 @@ class Radio extends React.Component<Props> implements Widget {
// Current versions of the radio widget always pass in the
// "correct" value through the choices. Old serialized state
// for radio widgets doesn't have this though, so we have to
// pull the correctness out of the review mode rubric. This
// pull the correctness out of the review mode scoring data. This
// only works because all of the places we use
// `restoreSerializedState()` also turn on reviewMode, but is
// fine for now.
Expand Down
Loading