Skip to content

fix: various metadata preservation issues (boxing, LaTeX-parsing) #212

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
42 changes: 28 additions & 14 deletions src/compute-engine/boxed-expression/box.ts
Original file line number Diff line number Diff line change
@@ -8,8 +8,16 @@ import {
CanonicalOptions,
} from './public';

import { Expression, MathJsonIdentifier } from '../../math-json/types';
import { machineValue, missingIfEmpty } from '../../math-json/utils';
import {
Expression,
ExpressionObject,
MathJsonIdentifier,
} from '../../math-json/types';
import {
hasMetaData,
machineValue,
missingIfEmpty,
} from '../../math-json/utils';
import {
isValidIdentifier,
validateIdentifier,
@@ -164,7 +172,7 @@ export function boxFunction(
return ce._fn(
name,
ops.map((x) => ce.box(x, { canonical: false })),
options.metadata
{ metadata: options.metadata }
);
}

@@ -384,16 +392,22 @@ export function box(
// Box a MathJSON object literal
//
if (typeof expr === 'object') {
const metadata = hasMetaData(expr as ExpressionObject)
? ({
latex: (expr as ExpressionObject).latex,
wikidata: (expr as ExpressionObject).wikidata,
} satisfies Metadata)
: undefined;
if ('fn' in expr) {
const [fnName, ...ops] = expr.fn;
return canonicalForm(
boxFunction(ce, fnName, ops, { canonical, structural }),
boxFunction(ce, fnName, ops, { canonical, structural, metadata }),
options.canonical!
);
}
if ('str' in expr) return new BoxedString(ce, expr.str);
if ('sym' in expr) return ce.symbol(expr.sym, { canonical });
if ('num' in expr) return ce.number(expr, { canonical });
if ('str' in expr) return new BoxedString(ce, expr.str, metadata);
if ('sym' in expr) return ce.symbol(expr.sym, { canonical, metadata });
if ('num' in expr) return ce.number(expr, { canonical, metadata });

throw new Error(`Unexpected MathJSON object: ${JSON.stringify(expr)}`);
}
@@ -481,7 +495,7 @@ function makeCanonicalFunction(
return ce._fn(
name,
validateArguments(ce, xs, def.signature, def.lazy, def.threadable) ?? xs,
metadata
{ metadata }
);
}

@@ -528,7 +542,7 @@ function makeCanonicalFunction(

// If we have some adjusted arguments, the arguments did not
// match the parameters of the signature. We're done.
if (adjustedArgs) return ce._fn(name, adjustedArgs, metadata);
if (adjustedArgs) return ce._fn(name, adjustedArgs, { metadata });

//
// 4/ Apply `idempotent` and `involution`
@@ -538,14 +552,14 @@ function makeCanonicalFunction(
if (def.involution) return args[0].op1;

// f(f(x)) -> f(x)
if (def.idempotent) return ce._fn(name, xs[0].ops!, metadata);
if (def.idempotent) return ce._fn(name, xs[0].ops!, { metadata });
}

//
// 5/ Sort the operands
//

return ce._fn(name, sortOperands(name, args), metadata);
return ce._fn(name, sortOperands(name, args), { metadata });
}

function makeNumericFunction(
@@ -581,7 +595,7 @@ function makeNumericFunction(

// If some of the arguments are not valid, we're done
// (note: the result is canonical, but not valid)
if (!ops.every((x) => x.isValid)) return ce._fn(name, ops, metadata);
if (!ops.every((x) => x.isValid)) return ce._fn(name, ops, { metadata });

//
// Short path for some functions
@@ -606,10 +620,10 @@ function makeNumericFunction(
// Ln(1) -> 0, Log(1) -> 0
if (ops[0].is(1)) return ce.Zero;
// Ln(a) -> Ln(a), Log(a) -> Log(a)
if (ops.length === 1) return ce._fn(name, ops, metadata);
if (ops.length === 1) return ce._fn(name, ops, { metadata });
}
// Ln(a,b) -> Log(a, b)
return ce._fn('Log', ops, metadata);
return ce._fn('Log', ops, { metadata });
}

return null;
8 changes: 3 additions & 5 deletions src/compute-engine/boxed-expression/public.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Complex } from 'complex-esm';
import type {
Attributes,
Expression,
MathJsonNumber,
MathJsonString,
@@ -2036,10 +2037,7 @@ export type EvaluateOptions = {
* @category Boxed Expression
*/

export type Metadata = {
latex?: string | undefined;
wikidata?: string | undefined;
};
export type Metadata = Pick<Attributes, 'latex' | 'wikidata'>;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

May wish to revert this: personally just thought it made sense.


/**
* When a unitless value is passed to or returned from a trigonometric function,
@@ -2277,7 +2275,7 @@ export interface IComputeEngine extends IBigNum {
_fn(
name: string,
ops: ReadonlyArray<BoxedExpression>,
options?: Metadata & { canonical?: boolean }
options?: { canonical?: boolean; metadata?: Metadata }
): BoxedExpression;

parse(
9 changes: 6 additions & 3 deletions src/compute-engine/compute-engine.ts
Original file line number Diff line number Diff line change
@@ -1924,7 +1924,7 @@ export class ComputeEngine implements IComputeEngine {
return this._fn(
'Rational',
[this.number(value[0]), this.number(value[1])],
{ ...metadata, canonical: false }
{ metadata, canonical: false }
);
}

@@ -2015,10 +2015,13 @@ export class ComputeEngine implements IComputeEngine {
_fn(
name: MathJsonIdentifier,
ops: ReadonlyArray<BoxedExpression>,
options?: Metadata & { canonical?: boolean }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Quite sure that that was a mistake, or at least non-desirable

options?: { canonical?: boolean; metadata?: Metadata }
): BoxedExpression {
const canonical = options?.canonical ?? true;
return new BoxedFunction(this, name, ops, { ...options, canonical });
return new BoxedFunction(this, name, ops, {
metadata: options?.metadata,
canonical,
});
}

/**
6 changes: 5 additions & 1 deletion src/compute-engine/latex-syntax/parse.ts
Original file line number Diff line number Diff line change
@@ -2046,7 +2046,11 @@ export class _Parser implements Parser {
} else if (typeof expr === 'number') {
expr = { latex, num: Number(expr).toString() };
} else if (typeof expr === 'string') {
expr = { latex, sym: expr };
if (expr.startsWith("'")) {
expr = { str: expr.slice(1, -1) };
} else {
expr = { sym: expr };
}
} else if (typeof expr === 'object' && expr !== null) {
(expr as ExpressionObject).latex = latex;
}
36 changes: 36 additions & 0 deletions src/math-json/utils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import type {
Attributes,
Expression,
ExpressionObject,
MathJsonFunction,
MathJsonIdentifier,
MathJsonNumber,
@@ -41,6 +43,40 @@ export function isFunctionObject(
return expr !== null && typeof expr === 'object' && 'fn' in expr;
}

export function isExpressionObject(
expr: Expression | null
): expr is ExpressionObject {
const isObj = expr !== null && typeof expr === 'object';
return (
isObj && ('fn' in expr || 'num' in expr || 'sym' in expr || 'str' in expr)
);
}

/**
* →true if Expression **expr** has *at least one* recognized meta-property key with its
* corresponding value non-'undefined'. Naturally, this also entails that *expr* is an
* 'ExpressionObject' variant of 'Expression'.
*
*/
export function hasMetaData(expr: Expression): expr is ExpressionObject {
return (
isExpressionObject(expr) &&
(hasMetaProperty(expr, 'latex') || hasMetaProperty(expr, 'wikidata'))
);
}

/**
* Returns true if meta prop. *key* is present in *expr* & this propery's value is also
* non-'undefined'.
*
*/
export function hasMetaProperty<
E extends ExpressionObject,
K extends keyof Pick<Attributes, 'latex' | 'wikidata'>,
>(expr: E, key: K): expr is E & { [k in K]: NonNullable<ExpressionObject[K]> } {
return expr[key] !== undefined;
}

/** If expr is a string literal, return it.
*
* A string literal is a JSON string that begins and ends with