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: correct aggregate params types #3846

Merged
merged 1 commit into from Jan 3, 2024
Merged

fix: correct aggregate params types #3846

merged 1 commit into from Jan 3, 2024

Conversation

domoritz
Copy link
Member

Follow up for #3686

@domoritz domoritz requested a review from a team as a code owner December 14, 2023 15:06
@julieg18
Copy link
Contributor

julieg18 commented Jan 2, 2024

Hello, is there anything blocking this PR from getting merged? Anything we can help with to move this forward?

@domoritz
Copy link
Member Author

domoritz commented Jan 3, 2024

@lsh can we include this in the next release?

@lsh
Copy link
Member

lsh commented Jan 3, 2024

@domoritz no complaints from me!

@domoritz
Copy link
Member Author

domoritz commented Jan 3, 2024

Please approve so we can merge.

@domoritz domoritz merged commit 2dd9e53 into main Jan 3, 2024
4 checks passed
@domoritz domoritz deleted the dom/agg_params branch January 3, 2024 04:48
@lsh lsh mentioned this pull request Jan 3, 2024
@julieg18
Copy link
Contributor

julieg18 commented Jan 17, 2024

Hello, thanks for releasing this fix in 5.27.0, it has helped me to progress on vega/vega-lite#9200. Would it be possible to update vega-cli, setting its required vega dependency version to be 5.27.0 instead of 5.26.1?

Since vega-lite installs vega through vega-cli, it's installing the version that still contains the types bug, causing the CLI to fail in my PR that adds aggregate params to vega-lite (vega/vega-lite#9225) with:

Error: src/compile/data/aggregate.ts(253,7): error TS2322: Type 'number[]' is not assignable to type 'object[]'.
  Type 'number' is not assignable to type 'object'.

If there is anything I can do to help, please let me know!

Edit: It looks like vega-lite's package.json may need to be updated as well since it would need to install the latest version of vega-cli even if vega-cli was updated.

@domoritz
Copy link
Member Author

@lsh is working on it.

@domoritz
Copy link
Member Author

#3862

@domoritz
Copy link
Member Author

@lsh made a release and is working on updating the dependency in Vega-Lite.

@lsh
Copy link
Member

lsh commented Jan 17, 2024

Merged into VL! Thanks for checking about this @julieg18!

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.

None yet

3 participants