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

Expressions API Updates #2399

Merged
merged 2 commits into from
Nov 11, 2023
Merged

Conversation

ada-x64
Copy link
Contributor

@ada-x64 ada-x64 commented Oct 25, 2023

Users can now specify expression columns using either an array of strings or an array of {name: string, expr: string} objects. This allows us to more easily rename expressions and separates parsing from the string type.
This API update is backwards-compatible, so plain-string expressions are still parsed correctly. Because of this I have not written any migration code.

In addition, I have removed some extra stuff from the column settings panel, including the attributes tab "type" field and the entire attributes tab if the column is not an expression.

@ada-x64 ada-x64 force-pushed the features/expr-alias-refactor branch 2 times, most recently from acb0642 to 390125c Compare October 31, 2023 15:17
@ada-x64 ada-x64 marked this pull request as draft October 31, 2023 19:01
@ada-x64
Copy link
Contributor Author

ada-x64 commented Oct 31, 2023

Converting this to draft for now. I would like to see the name field be optional so that we can write {expr: "'foo'"} for expressions that don't have a distinct name field. This declutters the config for larger unnamed expressions - which will be many of them once migrations happen. This idea does not map well to actual usage in the viewer, so I'm scrapping it for now.

@ada-x64 ada-x64 marked this pull request as ready for review November 6, 2023 16:53
@ada-x64 ada-x64 force-pushed the features/expr-alias-refactor branch from 390125c to 1fdb553 Compare November 6, 2023 18:05
@texodus
Copy link
Member

texodus commented Nov 11, 2023

Thanks for the PR!

I'm going to go ahead and merge this now as it looks good structurally and there is some backlog behind it. As discussed offline, I'd like to see some changes to both the UX and API before the next release:

  • Expresison name <input> should move to the attributes header
  • Should preserve default behavior of unnamed expressions are just named the expression itself (e.g. when the input is empty).
  • expressions key should be a dictionary.

@texodus texodus merged commit eab856d into finos:master Nov 11, 2023
12 checks passed
@texodus texodus added enhancement Feature requests or improvements breaking labels Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking enhancement Feature requests or improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants