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

Pass locale info through to vl-convert, default to display embed options when not set #3274

Merged
merged 5 commits into from
Nov 25, 2023

Conversation

jonmmease
Copy link
Contributor

This PR makes three changes related to locale handling:

  1. the formatLocale and timeFormatLocale embed options are passed through to vl-convert for static image export
  2. vl-convert is used to allow formatLocale and timeFormatLocale to be set using the locale name (e.g. it-IT) rather than only by setting the full locale dictionary.
  3. Chart saving defaults to using the Vega embed options set with alt.renderers.set_embed_options (if any). This way you can call alt.renderers.set_embed_options once and all of the charts will be displayed and saved using the provided locale configuration.

Still needs documentation, but I haven't decided whether to tackle that in this PR or not. If folks have ideas for where best to document this functionality let me know!

cc @MarcoBaroncini

@mattijn
Copy link
Contributor

mattijn commented Nov 23, 2023

Not sure if this is something to add to this PR, but still want to mention that a while ago I wished there was reset_embed_options() as I had changed the defaults but didn't know how to get back to the default...

@jonmmease
Copy link
Contributor Author

Yeah, it seems like set_embed_options persists across kernel restarts. Do you know how that works?

@mattijn
Copy link
Contributor

mattijn commented Nov 23, 2023

These comments from @domoritz around here: vega/vega-embed#170 (comment) might give more insight:

I'm somewhat hesitant to add this to Vega-Embed since formatting is a global property and not set per view.

@jheer do you have a suggestion for how we should handle formatting in Vega-Embed and Altair/Jupyter?

And

Great, this API should work. One issue I see now is that we set the locale globally. I wonder whether people would think that the locale is set per spec if it’s passed as an argument to embed when in fact it’s a global setting?

@MarcoBaroncini
Copy link

MarcoBaroncini commented Nov 24, 2023

Still needs documentation, but I haven't decided whether to tackle that in this PR or not. If folks have ideas for where best to document this functionality let me know!

I suggest to document this in Customizing Renderers chapter, maybe in a dedicated subchapter? I usually add documentation via different PR, but to me it makes no difference.

What should we add? I guess how to set locale for visualization via set_embed_options , and a section dedicated to saving via chart.save().

@jonmmease
Copy link
Contributor Author

@mattijn @MarcoBaroncini I added a new documentation section under "Customizing Visualizations". See if the description here makes sense. Note that I used a static SVG for the localization example chart output because sphinx_altair doesn't support localization yet, and even if it did it might interfere with the reset of the documentation.

I also added a changelog entry.

I think this PR is ready to go now!

@mattijn
Copy link
Contributor

mattijn commented Nov 25, 2023

Thanks @jonmmease, I've added a note how to set the default and changed javascript style variable names into the equivalent python style. Once the tests pass, we can merge this.

@mattijn mattijn merged commit aefce42 into main Nov 25, 2023
20 checks passed
@jonmmease
Copy link
Contributor Author

Thanks @mattijn!

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.

3 participants