Skip to content

Conversation

@Peter9192
Copy link
Member

@Peter9192 Peter9192 commented Apr 18, 2025

Skip BMI - it unnecessarily complicates things
Use runner from package instead

Update output variables in runner and use the metadata for nicer axis labels etc. in app

merge after #137

@Peter9192 Peter9192 mentioned this pull request Apr 22, 2025
@Peter9192 Peter9192 marked this pull request as ready for review April 22, 2025 08:52
@Peter9192 Peter9192 requested a review from sverhoeven April 22, 2025 21:13
@Peter9192 Peter9192 mentioned this pull request Apr 22, 2025
5 tasks
@Peter9192 Peter9192 changed the title Simplify CLASS Polish CLASS - drop BMI, update runner, add output metadata Apr 23, 2025
Moved docstring of  bmi.ts:BmiClass:run() to runClass
Copy link
Collaborator

@sverhoeven sverhoeven left a comment

Choose a reason for hiding this comment

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

Still works in app, cli and docs.

];

export type ClassOutput = {
[K in (typeof outputVariables)[number]["key"]]: number[];
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the idea of this is to have ClassOutput type be

type ClassOutput = {
    t: number[];
    h: number[];
    theta: number[];
    dtheta: number[];
    q: number[];
    dq: number[];
    dthetav: number[];
    we: number[];
    ws: number[];
    wthetave: number[];
    wthetav: number[];
}

however it is

type ClassOutput = {
    [x: string]: number[];
}

I quickly tried to typing outputVariables with as const satisfies readonly OutputVariable[], but that broke things downstream and did not have time to fix all those.

Base automatically changed from layout to main April 24, 2025 13:39
@Peter9192 Peter9192 merged commit 7bbeb86 into main Apr 24, 2025
4 checks passed
@Peter9192 Peter9192 deleted the update-class branch April 24, 2025 13:52
@Peter9192 Peter9192 mentioned this pull request Apr 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants