Skip to content

fix: add quotes around enum string values in visualization operators#4351

Merged
Xiao-zhen-Liu merged 11 commits intoapache:mainfrom
xuang7:fix/visualization-missing-quotes
Apr 23, 2026
Merged

fix: add quotes around enum string values in visualization operators#4351
Xiao-zhen-Liu merged 11 commits intoapache:mainfrom
xuang7:fix/visualization-missing-quotes

Conversation

@xuang7
Copy link
Copy Markdown
Contributor

@xuang7 xuang7 commented Apr 6, 2026

What changes were proposed in this PR?

This PR adds quotes around plain String values returned by Java enum methods in three visualization operators so the generated Python code is valid:

  • ContinuousErrorBands: mode='${bandConf.mode.getModeInPlotly}'
  • ContourPlot: contours_coloring='${coloringMethod.getColoringMethod}'
  • Histogram2D: histnorm='${normalize.getValue}'

Any related issues, documentation, discussions?

Related to #4189
Resolves #4350

How was this PR tested?

Existing tests passed

Was this PR authored or co-authored using generative AI tooling?

No

@chenlica
Copy link
Copy Markdown
Contributor

chenlica commented Apr 7, 2026

Can we add test cases to catch this problem?

@xuang7
Copy link
Copy Markdown
Contributor Author

xuang7 commented Apr 21, 2026

Can we add test cases to catch this problem?

Discussed this with @carloea2, please correct me if I missed anything. We already have PythonCodeRawInvalidTextSpec, which runs py_compile on all generated operator code, but it can't catch this bug. mode=lines is syntactically valid Python, so it only fails at runtime with a NameError. A test that would catch this issue would need to execute the generated code with real data and Plotly imported. I'm not sure the added test complexity is worth it right now. Could we merge this first to unblock the broken operators, and then follow up with a more meaningful integration test if needed?

@chenlica
Copy link
Copy Markdown
Contributor

@carloea2 Please chime in and review it. After that, @Xiao-zhen-Liu can review it.

@chenlica chenlica requested a review from Xiao-zhen-Liu April 22, 2026 06:13
@carloea2
Copy link
Copy Markdown
Contributor

LGTM.

I think executing the operators in the specs to catch runtime errors can be a big improvement but requires careful consideration and this PR should not be blocked by that.

Thanks. I hope this PR can be merged soon.

Copy link
Copy Markdown
Contributor

@Xiao-zhen-Liu Xiao-zhen-Liu left a comment

Choose a reason for hiding this comment

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

LGTM

@carloea2
Copy link
Copy Markdown
Contributor

Can we merge it?

@Xiao-zhen-Liu Xiao-zhen-Liu enabled auto-merge (squash) April 23, 2026 05:06
@chenlica
Copy link
Copy Markdown
Contributor

I will do the merge after test cases pass.

@Xiao-zhen-Liu Xiao-zhen-Liu merged commit 0316204 into apache:main Apr 23, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Visualization operators fail due to unquoted enum string values

5 participants