Skip to content

MAINT: vendor vega assets - #94

Merged
lizgehret merged 14 commits into
qiime2:devfrom
johnchase:agent/vendor-vega-assets
Sep 3, 2026
Merged

MAINT: vendor vega assets#94
lizgehret merged 14 commits into
qiime2:devfrom
johnchase:agent/vendor-vega-assets

Conversation

@johnchase

@johnchase johnchase commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Description

  • Fixes issue MAINT: replace cdn links in vega specs.  #1 by removing jsDelivr runtime dependencies.
  • Vendors Vega 5.22.1 and vega-embed 6.20.8.
  • Copies both JavaScript bundles and their licenses into every generated visualization.
  • Applies to scatterplot, heatmap, lineplot, and boxplot outputs.
  • Adds byte-for-byte output verification and pinned SHA-256 tests.

AI Disclosure

  • NO AI USED.
  • AI USED.

AI Usage Details

Tool: Codex, using GPT-5.
Codex inspected the issue and contribution guidance.
Codex produced all new Python implementation, tests, and vendor documentation.
Codex downloaded the pinned upstream Vega assets, verified checksums, ran tests/linting, and pushed the commits.
User independently tested plugin, verified visualization, tested offline usage.
User defined implementation details such as package structure and naming
User independently reviewed PR

@johnchase johnchase changed the title Agent/vendor vega assets MAINT: vendor vega assets Aug 11, 2026
@lizgehret lizgehret self-assigned this Aug 17, 2026

@lizgehret lizgehret left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hey @johnchase thanks for this contribution, this had been on my list for quite some time!

Initial review looks good with a few small formatting requests. I'm going to do some local testing/poking around; I'll follow up in the next couple of days if anything else is needed before getting this merged!

Comment thread q2_vizard/tests/test_util.py Outdated
Comment thread q2_vizard/boxplot.py Outdated
Comment thread q2_vizard/heatmap.py Outdated
Comment thread q2_vizard/lineplot.py Outdated
Comment thread q2_vizard/_assets.py Outdated
Comment thread q2_vizard/scatterplot.py Outdated
@lizgehret

Copy link
Copy Markdown
Member

Hey @johnchase I'm going to go ahead and implement these suggestions to save you some time (assuming you won't mind)

@lizgehret

Copy link
Copy Markdown
Member

Another follow-up; I did a bit more refactoring to the viz rendering that will make it easier to refactor the asset tests so we aren't hard coding the visualizations that currently exist (and rather are doing these checks against all available visualizations at any point in time). I'm going to finish this up tomorrow and then will have @misialq review!

Comment thread q2_vizard/_examples.py
@lizgehret lizgehret assigned misialq and unassigned lizgehret Aug 28, 2026
@lizgehret
lizgehret requested a review from misialq August 28, 2026 10:01
@lizgehret

Copy link
Copy Markdown
Member

Okay refactor is all finished and ready for your review @misialq 🙂

@misialq misialq left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM @lizgehret! 🎉 (I tested using the boxplot visualization)

Comment thread q2_vizard/assets/vendor/README.md Outdated
@misialq misialq assigned lizgehret and unassigned misialq Sep 3, 2026
@lizgehret

Copy link
Copy Markdown
Member

Note for posterity (moving from the vendor README):

As of these PR changes, the size of each .qzv generated will now increase in size by 194.6 KB (when compressed).
This number may change in the future if/when the version of vega being used gets bumped.

@lizgehret
lizgehret merged commit 09e1ad9 into qiime2:dev Sep 3, 2026
5 checks passed
@lizgehret lizgehret removed their assignment Sep 3, 2026
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.

5 participants