Skip to content

IMP: expose colorPalette parameter for all visualizers - #112

Merged
misialq merged 14 commits into
devfrom
expose-colorPalette-param-lineplot
Oct 16, 2026
Merged

misialq merged 14 commits into
devfrom
expose-colorPalette-param-lineplot

Conversation

@lizgehret

@lizgehret lizgehret commented Sep 25, 2026 •

Copy link
Copy Markdown
Member

Description

Closes #103. Exposes the colorPalette parameter for all visualizations; discrete & continuous palettes implemented for scatterplot(s).

AI Disclosure

  • NO AI USED.
  • AI USED.

AI Usage Details

Claude Opus 5.5 was used to refactor the existing selenium tests to account for the additional parameters and desired test cases, reviewed by me. All new (non-selenium) tests & implementation/refactoring done by me.

@lizgehret

Copy link
Copy Markdown
Member Author

Hey @VinzentRisch, I'm just going to expose this for all viz's (just to improve developer customization for all of the plots). I'll do this and add a few small tests asserting color choice and then this should be ready for your review (if you don't mind) 🙂

@lizgehret lizgehret self-assigned this Sep 25, 2026
@lizgehret

lizgehret commented Sep 30, 2026 •

Copy link
Copy Markdown
Member Author

Hey @VinzentRisch this is ready for a review 🙂

Apologies in advance; the test changes needed for this were fairly substantial. Some refactoring was required to make the selenium tests slightly less of a monstrosity; I opened up an issue for myself to refactor these further to pull out what checks aren't strictly browser-required, but that was out of scope for this PR.

@lizgehret lizgehret changed the title IMP: expose colorPalette parameter IMP: expose colorPalette parameter for all visualizers Sep 30, 2026
@lizgehret lizgehret assigned VinzentRisch and unassigned lizgehret Sep 30, 2026
@VinzentRisch

Copy link
Copy Markdown
Contributor

Hey @lizgehret
I don't think I will have time to review this today and I am gone until Wednesday next week.
I will do it then if that is okay. if you want it reviewed earlier you have to give it to someone else.

@lizgehret

Copy link
Copy Markdown
Member Author

Hey @lizgehret

I don't think I will have time to review this today and I am gone until Wednesday next week.

I will do it then if that is okay. if you want it reviewed earlier you have to give it to someone else.

All good, thanks @VinzentRisch! I'll see if @misialq has time before then, otherwise when you get back is great 🙂

@ebolyen ebolyen assigned misialq and unassigned VinzentRisch Oct 5, 2026

@misialq misialq left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey @lizgehret, this all looks pretty good to me - I just have some wording update suggestions below, let me know what you think! I did try it with some plots and it all seemed to work as expected 🚀

Comment thread q2_vizard/plugin_setup.py Resolved
Comment thread q2_vizard/scatterplot.py Resolved
Comment thread q2_vizard/plugin_setup.py Resolved
@lizgehret

Copy link
Copy Markdown
Member Author

Thanks for the feedback @misialq! This should be ready for a final review 🙂

@misialq misialq left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM! 🚀

@misialq
misialq merged commit 61ec3ee into dev Oct 16, 2026
5 checks passed
@misialq
misialq deleted the expose-colorPalette-param-lineplot branch October 16, 2026 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects
Status: No status

Development

Successfully merging this pull request may close these issues.

IMP: Expose colorPalette directly to the lineplot vis

3 participants