Skip to content

Commit

Permalink
fix: add inputId property to InputSpec interface (#321)
Browse files Browse the repository at this point in the history
Fixes #320
  • Loading branch information
chrispcampbell authored May 3, 2023
1 parent 42b5e0f commit f461433
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 10 deletions.
13 changes: 13 additions & 0 deletions packages/build/docs/interfaces/InputSpec.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,19 @@ Describes a model input variable.

## Properties

### inputId

`Optional` **inputId**: `string`

The stable input identifier. It is recommended to set this to a value (for example, a
numeric string like what `plugin-config` uses) that is separate from `varName` and is
stable between two versions of the model. This way, if an input variable is renamed
between two versions of the model, comparisons can still be performed between the two.
If a distinct `inputId` is not available, plugins can infer one from `varName`, but
note that this approach will be less resilient to renames.

___

### varName

**varName**: `string`
Expand Down
10 changes: 10 additions & 0 deletions packages/build/src/_shared/model-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,16 @@
* Describes a model input variable.
*/
export interface InputSpec {
/**
* The stable input identifier. It is recommended to set this to a value (for example, a
* numeric string like what `plugin-config` uses) that is separate from `varName` and is
* stable between two versions of the model. This way, if an input variable is renamed
* between two versions of the model, comparisons can still be performed between the two.
* If a distinct `inputId` is not available, plugins can infer one from `varName`, but
* note that this approach will be less resilient to renames.
*/
inputId?: string

/** The variable name (as used in the modeling tool). */
varName: string

Expand Down
9 changes: 8 additions & 1 deletion packages/plugin-config/src/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,13 @@ export class ConfigContext {
this.buildContext.writeStagedFile(srcDir, dstDir, filename, content)
}

addInputVariable(inputVarName: string, defaultValue: number, minValue: number, maxValue: number): void {
addInputVariable(
inputId: string,
inputVarName: string,
defaultValue: number,
minValue: number,
maxValue: number
): void {
// We use the C name as the key to avoid redundant entries in cases where
// the csv file refers to variables with different capitalization
const varId = sdeNameForVensimVarName(inputVarName)
Expand All @@ -79,6 +85,7 @@ export class ConfigContext {
console.error(`ERROR: Input variable ${inputVarName} was already added`)
}
this.inputSpecs.set(varId, {
inputId,
varName: inputVarName,
defaultValue,
minValue,
Expand Down
4 changes: 2 additions & 2 deletions packages/plugin-config/src/gen-inputs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ function inputSpecFromCsv(r: CsvRow, context: ConfigContext): InputSpec | undefi
e += `default=${defaultValue} min=${minValue} max=${maxValue}`
throw new Error(e)
}
context.addInputVariable(varName, defaultValue, minValue, maxValue)
context.addInputVariable(inputId, varName, defaultValue, minValue, maxValue)

const format = optionalString(r['format']) || '.0f'

Expand Down Expand Up @@ -179,7 +179,7 @@ function inputSpecFromCsv(r: CsvRow, context: ConfigContext): InputSpec | undefi

const minValue = Math.min(offValue, onValue)
const maxValue = Math.max(offValue, onValue)
context.addInputVariable(varName, defaultValue, minValue, maxValue)
context.addInputVariable(inputId, varName, defaultValue, minValue, maxValue)

// The `controlled input ids` field dictates which rows are active
// when this switch is on or off. Examples of the format of this field:
Expand Down
55 changes: 48 additions & 7 deletions packages/plugin-config/src/processor.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { fileURLToPath } from 'url'
import temp from 'temp'
import { afterAll, beforeAll, describe, expect, it } from 'vitest'

import type { BuildOptions, UserConfig } from '@sdeverywhere/build'
import type { BuildOptions, ModelSpec, Plugin, UserConfig } from '@sdeverywhere/build'
import { build } from '@sdeverywhere/build'

import type { ConfigOptions } from './processor'
Expand All @@ -22,7 +22,10 @@ interface TestEnv {
buildOptions: BuildOptions
}

async function prepareForBuild(optionsFunc: (corePkgDir: string) => ConfigOptions): Promise<TestEnv> {
async function prepareForBuild(
optionsFunc: (corePkgDir: string) => ConfigOptions,
plugins: Plugin[] = []
): Promise<TestEnv> {
const baseTmpDir = await temp.mkdir('sde-plugin-config')
const projDir = joinPath(baseTmpDir, 'proj')
await mkdir(projDir)
Expand All @@ -32,7 +35,8 @@ async function prepareForBuild(optionsFunc: (corePkgDir: string) => ConfigOption
const config: UserConfig = {
rootDir: projDir,
modelFiles: [],
modelSpec: configProcessor(optionsFunc(corePkgDir))
modelSpec: configProcessor(optionsFunc(corePkgDir)),
plugins
}

const buildOptions: BuildOptions = {
Expand Down Expand Up @@ -216,16 +220,53 @@ describe('configProcessor', () => {
})

it('should write to default directory structure if single out dir is provided', async () => {
let capturedModelSpec: ModelSpec
const plugin: Plugin = {
async preGenerate(_, modelSpec) {
capturedModelSpec = modelSpec
}
}

const configDir = joinPath(__dirname, '__tests__', 'config1')
const testEnv = await prepareForBuild(corePkgDir => ({
config: configDir,
out: corePkgDir
}))
const testEnv = await prepareForBuild(
corePkgDir => ({
config: configDir,
out: corePkgDir
}),
[plugin]
)
const result = await build('production', testEnv.buildOptions)
if (result.isErr()) {
throw new Error('Expected ok result but got: ' + result.error.message)
}

expect(capturedModelSpec).toBeDefined()
expect(capturedModelSpec.inputs).toEqual([
{
inputId: '1',
varName: 'Input A',
defaultValue: 0,
minValue: -50,
maxValue: 50
},
{
inputId: '2',
varName: 'Input B',
defaultValue: 0,
minValue: -50,
maxValue: 50
},
{
inputId: '3',
varName: 'Input C',
defaultValue: 0,
minValue: 0,
maxValue: 1
}
])
expect(capturedModelSpec.outputs).toEqual([{ varName: 'Var 1' }])
expect(capturedModelSpec.datFiles).toEqual(['../Data1.dat', '../Data2.dat'])

const specJsonFile = joinPath(testEnv.projDir, 'sde-prep', 'spec.json')
expect(await readFile(specJsonFile, 'utf8')).toEqual(specJson1)

Expand Down

0 comments on commit f461433

Please sign in to comment.