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

fix: add inputId property to InputSpec interface #321

Merged
merged 2 commits into from
May 3, 2023
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
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