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.

Sign up for GitHub

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

Jump to bottom

Replace units.yaml with new parameter definition logic #418

Merged
merged 65 commits into from
Feb 1, 2024
Merged

Replace units.yaml with new parameter definition logic #418

merged 65 commits into from
Feb 1, 2024

Conversation

jmcook1186
Copy link
Contributor

@jmcook1186 jmcook1186 commented Jan 30, 2024 •
edited

Types of changes

  • Enhancement (project structure, spelling, grammar, formatting)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

A description of the changes proposed in the Pull Request

This PR removes all logic related to units.ts and untis.yml and instead:

  • uses parameters.ts to export canonical parameter definitions
  • allows users to add new parameters to the canonical set via the manifest file
  • allows users to override the canonical set by providing their own file (.ts, .js or .json)
  • provides necessary validation and warning logs
  • updates tests and fixes breakages caused by logic change

This does NOT yet pull the necessary file from an npm package - instead it uses a local param.ts file in place of the old units.yml. The user can append to that file via the manifest file or provide a completely separate one via the CLI.

closes #416 and #403

@jmcook1186
[cli]: initial commit for units experiment
12b06e5
@jmcook1186
[cli]: implement basic logic for adding params to units.yml
ec6a30c
@jmcook1186
[cli]: swap units.yaml for units.json
0b88c18
@jmcook1186
[cli]: add todo comment for fixing validation
16d2c86
@jmcook1186
[cli]: add CLI overrideParams command
2a3787a
@jmcook1186
[cli]: add validation logic for param field in impl
04c3ac3
@jmcook1186
[chore]: cleanup and lint current implementation
05bc1c6
@jmcook1186
[cli]: strip out unitkeys validation logic
bd8f88e
@jmcook1186
[cli] add warning log on override flag
4565451
@jmcook1186
cli: rm unitkeys check from modelParams
dfc7956
@jmcook1186
[test] add test impl
f80d09c
@jmcook1186
[cli] make units.json -> params.ts
3693f45
@jmcook1186
[tests] fix unit tests broken by rm Unitkeys
93adc03
@jmcook1186
[tests] fix import path to resolve last outstanding test failure
ac2fe1a
@jmcook1186
[lint] run linter
3e58998
@jmcook1186
cli: update unitsDealer for new units logic
6f31486
@jmcook1186
[lint]: run linter
8247349
@jmcook1186
[tests] rm redundant unitsDealer test
b2cfaf6
@jmcook1186 jmcook1186 marked this pull request as ready for review January 30, 2024 15:17
narekhovhannisyan
narekhovhannisyan reviewed Jan 31, 2024
View reviewed changes
package.json Outdated Show resolved Hide resolved
narekhovhannisyan
narekhovhannisyan reviewed Jan 31, 2024
View reviewed changes
src/index.ts Outdated Show resolved Hide resolved
jmcook1186 and others added 27 commits January 31, 2024 16:40
@jmcook1186
cli: minor cleanup in index.ts
b7deb15
@jmcook1186
cli: fix prettified json dump to console
d5bcab9
@narekhovhannisyan
package: remove models dependency.
d48b428
@narekhovhannisyan
util: tune params.aggregation type in validator.
b1d31de
@narekhovhannisyan
types: tune units.
4b23f9a
@narekhovhannisyan
types: use parameter array for params.
8d49b1c
@narekhovhannisyan
lib: in supercomputer fallthrough params type.
438c6d0
@narekhovhannisyan
src: in index optimize strategy.
522c078
@narekhovhannisyan
config: fix letter case for aggregation in params.
e4e9d35
@narekhovhannisyan
util: use units type for selectors.
0fedd7b
@narekhovhannisyan
types: in units drop parameters dependency.
79e94d6
@narekhovhannisyan
types: in impl use string instead of parameter key.
30a6005
@narekhovhannisyan
models: in time-sync drop parameters type dependency.
bbccfd9
@narekhovhannisyan
util: use parameters type for selectors.
0d4eefe
@narekhovhannisyan
types: use parameters term instead of units.
f494570
@narekhovhannisyan
util: use string as atype for metrics.
45e4dfa
@narekhovhannisyan
util: drop union helpres from validation.
49d9146
@narekhovhannisyan
util: fix import path for parameters in selectors.
852bad4
@narekhovhannisyan
types: tune impl parameters.
643a01a
@narekhovhannisyan
types: tune types for aggregator.
f716223
@narekhovhannisyan
lib: in supercomputer refactor overrideOrAppendParams.
a738c4a
@narekhovhannisyan
lib: update types in aggregator.
b77c532
@narekhovhannisyan
config: update import path for parameters.
867b8d7
@narekhovhannisyan
tests: update types in param selectors.
3096a17
@narekhovhannisyan
tests: update types in param supercomputer.
74bd765
@narekhovhannisyan
tests: update types in param aggregator.
6e32667
@narekhovhannisyan
src: optimize index file.
7df291f
@jmcook1186 jmcook1186 merged commit e9be9ff into dev Feb 1, 2024
2 checks passed
@jmcook1186 jmcook1186 deleted the units-logic-update branch February 1, 2024 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@narekhovhannisyan narekhovhannisyan narekhovhannisyan approved these changes

Assignees
No one assigned
Labels
None yet
Projects
IF
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove reliance on Units.ts and UnitKeyName
2 participants
@jmcook1186 @narekhovhannisyan