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

Added optional objects to input parameters #1437

Merged
merged 9 commits into from
Sep 3, 2024

Conversation

cohansen
Copy link
Contributor

@cohansen cohansen commented Aug 20, 2024

Closes #1398.

Adds an optional object to input parameters. Example:

@INPUT_PARAMS testparam { "TYPE": "FLOAT", "RANGE": "1, 2, 135.0..." } asd asd2

Thanks for the grammar help @goetzrrGit !

@cohansen cohansen self-assigned this Aug 20, 2024
@cohansen cohansen requested a review from a team as a code owner August 20, 2024 20:36
@goetzrrGit
Copy link
Contributor

@cohansen We probably want to update the core linter since we support this grammar change in the SeqN grammar core.

@cohansen
Copy link
Contributor Author

@cohansen We probably want to update the core linter since we support this grammar change in the SeqN grammar core.

I added the object check in the sequence linter, were you talking about somewhere else too?

@goetzrrGit
Copy link
Contributor

goetzrrGit commented Aug 21, 2024

Wow did not see that :P

The only other place is to update the to-seqjson as we can fill out the rest of the missing information defined by the schema since we didn't have this before.

https://github.com/NASA-AMMOS/seq-json-schema/blob/fdad4b3af45906de9c02a3fb67817f1e399fde69/schema.json#L535-L574

You can use this as a reference

https://github.com/NASA-AMMOS/aerie/blob/develop/sequencing-server/src/lib/codegen/CommandEDSLPreface.ts#L1003-L1033

Also the from-seqjson might need to be updated as well.

@cohansen cohansen force-pushed the feature/sequence-workspaces branch from cfcd13b to 2ddc536 Compare August 21, 2024 19:50
Copy link
Contributor

@goetzrrGit goetzrrGit left a comment

Choose a reason for hiding this comment

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

The code looks good so far. I think we should add some linter checks to make sure the seqJSON is valid when it is generated. Also, I don't see a from-seqjson which we need to support roundtripping.

src/utilities/sequence-editor/to-seq-json.ts Outdated Show resolved Hide resolved
src/utilities/sequence-editor/to-seq-json.ts Show resolved Hide resolved
Copy link
Contributor

@goetzrrGit goetzrrGit left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for adding the roundtriping.

@cohansen cohansen force-pushed the feature/update-input-params branch from cb11ac4 to 98d6421 Compare August 27, 2024 16:30
@duranb duranb force-pushed the feature/sequence-workspaces branch from 2ddc536 to 2576f0f Compare August 29, 2024 17:09
@cohansen cohansen force-pushed the feature/sequence-workspaces branch from 2576f0f to f4d4704 Compare August 30, 2024 18:01
Base automatically changed from feature/sequence-workspaces to develop August 30, 2024 19:04
@cohansen cohansen force-pushed the feature/update-input-params branch from 98d6421 to 4c356a1 Compare September 3, 2024 16:16
@cohansen cohansen merged commit dbccc45 into develop Sep 3, 2024
5 checks passed
@cohansen cohansen deleted the feature/update-input-params branch September 3, 2024 17:51
JosephVolosin pushed a commit that referenced this pull request Oct 21, 2024
* Rough prototype of workspaces

* Added the sequence header back in

* More workspace improvements, navigation and persistance working now

* Added optional objects to input parameters

* Added the parsed input_parameter fields to seqjson

* Added linting rules for input parameter objects

* Added input param metadata parsing to from-seq-json

* Fixed a duplicated style in the SequenceTable

* Fixed 2 from-seq-json unit tests
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.

Phoenix: Add support for global and local variables, and sequence input parameters
2 participants