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

Move scoring logic: Matrix #2116

Merged
merged 2 commits into from
Jan 17, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {getDecimalSeparator} from "@khanacademy/perseus-core";

import {MathFieldActionType} from "../../types";
import {getDecimalSeparator} from "../../utils";
import {mathQuillInstance} from "../input/mathquill-instance";

import handleArrow from "./handle-arrow";
Expand Down
7 changes: 2 additions & 5 deletions packages/math-input/src/components/keypad/button-assets.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ asset.
In the future it would be great if these were included from files so that
no copying and pasting is necessary.
*/
import {getDecimalSeparator} from "@khanacademy/perseus-core";
import * as React from "react";

import {DecimalSeparator, getDecimalSeparator} from "../../utils";
import {useMathInputI18n} from "../i18n-context";

import type Key from "../../data/keys";
Expand Down Expand Up @@ -176,10 +176,7 @@ export default function ButtonAsset({id}: Props): React.ReactNode {
case "PERIOD":
// Different locales use different symbols for the decimal separator
// (, vs .)
if (
id === "DECIMAL" &&
getDecimalSeparator(locale) === DecimalSeparator.COMMA
) {
if (id === "DECIMAL" && getDecimalSeparator(locale) !== ".") {
// comma decimal separator
return (
<svg
Expand Down
37 changes: 0 additions & 37 deletions packages/math-input/src/utils.ts
Original file line number Diff line number Diff line change
@@ -1,40 +1,3 @@
export const DecimalSeparator = {
COMMA: ",",
PERIOD: ".",
} as const;

/**
* Get the character used for separating decimals.
*/
export const getDecimalSeparator = (locale: string): string => {
let separator: string = DecimalSeparator.PERIOD;

switch (locale) {
// TODO(somewhatabstract): Remove this when Chrome supports the `ka`
// locale properly.
// https://github.com/formatjs/formatjs/issues/1526#issuecomment-559891201
//
// Supported locales in Chrome:
// https://source.chromium.org/chromium/chromium/src/+/master:third_party/icu/scripts/chrome_ui_languages.list
case "ka":
separator = ",";
break;

default:
const numberWithDecimalSeparator = 1.1;
// TODO(FEI-3647): Update to use .formatToParts() once we no longer have to
// support Safari 12.
const match = new Intl.NumberFormat(locale)
.format(numberWithDecimalSeparator)
// 0x661 is ARABIC-INDIC DIGIT ONE
// 0x6F1 is EXTENDED ARABIC-INDIC DIGIT ONE
.match(/[^\d\u0661\u06F1]/);
separator = match?.[0] ?? ".";
}

return separator === "," ? DecimalSeparator.COMMA : DecimalSeparator.PERIOD;
};

const CDOT_ONLY = [
"az",
"cs",
Expand Down
2 changes: 2 additions & 0 deletions packages/perseus-core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ export type {ErrorKind} from "./error/errors";

// Careful, `version.ts` uses this function so it _must_ be imported above it
export {addLibraryVersionToPerseusDebug} from "./utils/add-library-version-to-perseus-debug";
export {default as getMatrixSize} from "./utils/get-matrix-size";
export {default as getDecimalSeparator} from "./utils/get-decimal-separator";

export {libVersion} from "./version";

Expand Down
27 changes: 27 additions & 0 deletions packages/perseus-core/src/utils/get-matrix-size.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import _ from "underscore";

function getMatrixSize(matrix: ReadonlyArray<ReadonlyArray<number>>) {
const matrixSize = [1, 1];

// We need to find the widest row and tallest column to get the correct
// matrix size.
_(matrix).each((matrixRow, row) => {
let rowWidth = 0;
_(matrixRow).each((matrixCol, col) => {
if (matrixCol != null && matrixCol.toString().length) {
rowWidth = col + 1;
}
});

// Matrix width:
matrixSize[1] = Math.max(matrixSize[1], rowWidth);

// Matrix height:
if (rowWidth > 0) {
matrixSize[0] = Math.max(matrixSize[0], row + 1);
}
});
return matrixSize;
}

export default getMatrixSize;
25 changes: 1 addition & 24 deletions packages/perseus-editor/src/widgets/matrix-editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
EditorJsonify,
MatrixWidget,
} from "@khanacademy/perseus";
import {getMatrixSize} from "@khanacademy/perseus-core";
import PropTypes from "prop-types";
import * as React from "react";
import _ from "underscore";
Expand All @@ -17,30 +18,6 @@ const Matrix = MatrixWidget.widget;
// have to cap it at some point.
const MAX_BOARD_SIZE = 6;

const getMatrixSize = function (matrix: any) {
const matrixSize = [1, 1];

// We need to find the widest row and tallest column to get the correct
// matrix size.
_(matrix).each((matrixRow, row) => {
let rowWidth = 0;
_(matrixRow).each((matrixCol, col) => {
if (matrixCol != null && matrixCol.toString().length) {
rowWidth = col + 1;
}
});

// Matrix width:
matrixSize[1] = Math.max(matrixSize[1], rowWidth);

// Matrix height:
if (rowWidth > 0) {
matrixSize[0] = Math.max(matrixSize[0], row + 1);
}
});
return matrixSize;
};

type Props = any;

class MatrixEditor extends React.Component<Props> {
Expand Down
2 changes: 2 additions & 0 deletions packages/perseus-score/src/error-codes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const MULTIPLICATION_SIGN_ERROR = "MULTIPLICATION_SIGN_ERROR";
const INVALID_SELECTION_ERROR = "INVALID_SELECTION_ERROR";
const CHOOSE_CORRECT_NUM_ERROR = "CHOOSE_CORRECT_NUM_ERROR";
const NOT_NONE_ABOVE_ERROR = "NOT_NONE_ABOVE_ERROR";
const FILL_ALL_CELLS_ERROR = "FILL_ALL_CELLS_ERROR";

const ErrorCodes = {
MISSING_PERCENT_ERROR,
Expand All @@ -20,6 +21,7 @@ const ErrorCodes = {
INVALID_SELECTION_ERROR,
CHOOSE_CORRECT_NUM_ERROR,
NOT_NONE_ABOVE_ERROR,
FILL_ALL_CELLS_ERROR,
};

export default ErrorCodes;
2 changes: 2 additions & 0 deletions packages/perseus-score/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@ export type * from "./validation.types";
export {default as scoreCategorizer} from "./widgets/categorizer/score-categorizer";
export {default as scoreCSProgram} from "./widgets/cs-program/score-cs-program";
export {default as scoreDropdown} from "./widgets/dropdown/score-dropdown";
export {default as scoreExpression} from "./widgets/expression/score-expression";
export {default as scoreIframe} from "./widgets/iframe/score-iframe";
export {
default as scoreLabelImage,
labelImageScoreMarker,
} from "./widgets/label-image/score-label-image";
export {default as scoreMatcher} from "./widgets/matcher/score-matcher";
export {default as scoreMatrix} from "./widgets/matrix/score-matrix";
export {default as scoreNumberLine} from "./widgets/number-line/score-number-line";
export {default as scoreNumericInput} from "./widgets/numeric-input/score-numeric-input";
export {default as scoreOrderer} from "./widgets/orderer/score-orderer";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import {mockStrings} from "../../strings";

import {expressionItem3Options} from "./expression.testdata";
import scoreExpression from "./score-expression";
import {expressionItem3Options} from "./score-expression.testdata";
import * as ExpressionValidator from "./validate-expression";

import type {PerseusExpressionRubric} from "@khanacademy/perseus-score";

// TODO: remove strings as a param for scorers
const mockStrings = {};

describe("scoreExpression", () => {
it("should be correctly answerable if validation passes", function () {
// Arrange
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import type {PerseusExpressionWidgetOptions} from "@khanacademy/perseus-core";

export const expressionItem3Options: PerseusExpressionWidgetOptions = {
answerForms: [
{
considered: "ungraded",
form: false,
simplify: false,
value: "x+1",
},
{
considered: "wrong",
form: false,
simplify: false,
value: "y+1",
},
{
considered: "correct",
form: false,
simplify: false,
value: "z+1",
},
{
considered: "correct",
form: false,
simplify: false,
value: "a+1",
},
],
times: false,
buttonSets: ["basic"],
functions: ["f", "g", "h"],
buttonsVisible: "focused",
visibleLabel: "number of cm",
ariaLabel: "number of centimeters",
};
Original file line number Diff line number Diff line change
@@ -1,21 +1,22 @@
import * as KAS from "@khanacademy/kas";
import {Errors} from "@khanacademy/perseus-core";
import {KhanAnswerTypes} from "@khanacademy/perseus-score";
import {
Errors,
getDecimalSeparator,
PerseusError,
} from "@khanacademy/perseus-core";
import _ from "underscore";

import {Log} from "../../logging/log";
import KhanAnswerTypes from "../../util/answer-types";

import getDecimalSeparator from "./get-decimal-separator";
import validateExpression from "./validate-expression";

import type {PerseusStrings} from "../../strings";
import type {PerseusExpressionAnswerForm} from "@khanacademy/perseus-core";
import type {Score} from "../../util/answer-types";
import type {
PerseusScore,
Score,
PerseusExpressionRubric,
PerseusExpressionUserInput,
} from "@khanacademy/perseus-score";
PerseusScore,
} from "../../validation.types";
import type {PerseusExpressionAnswerForm} from "@khanacademy/perseus-core";

/* Content creators input a list of answers which are matched from top to
* bottom. The intent is that they can include spcific solutions which should
Expand All @@ -38,7 +39,8 @@ import type {
function scoreExpression(
userInput: PerseusExpressionUserInput,
rubric: PerseusExpressionRubric,
strings: PerseusStrings,
// TODO: remove strings as a param for scorers
strings: unknown,
locale: string,
): PerseusScore {
const validationError = validateExpression(userInput);
Expand All @@ -62,12 +64,10 @@ function scoreExpression(
// in the function variables list for the expression.
if (!expression.parsed) {
/* c8 ignore next */
Log.error(
throw new PerseusError(
"Unable to parse solution answer for expression",
Errors.InvalidInput,
{loggedMetadata: {rubric: JSON.stringify(rubric)}},
);
return null;
}

return KhanAnswerTypes.expression.createValidatorFunctional(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type {
ValidationResult,
PerseusExpressionUserInput,
} from "@khanacademy/perseus-score";
ValidationResult,
} from "../../validation.types";

/**
* Checks user input from the expression widget to see if it is scorable.
Expand Down
Loading
Loading