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 silent LaTeXString conversion #2588

Merged
merged 4 commits into from
Feb 7, 2023
Merged

Conversation

t-bltg
Copy link
Collaborator

@t-bltg t-bltg commented Jan 12, 2023

Description

Fix #2513.

In OP's bug report, the LaTeXString was silently converted to a String, resulting in $ signs appearing in the Axis3D case.

Type of change

Delete options that do not apply:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

  • Added an entry in NEWS.md (for new features and breaking changes)
  • Added or changed relevant sections in the documentation
  • Added unit tests for new algorithms, conversion methods, etc.
  • Added reference image tests for new plotting functions, recipes, visual options, etc.

@t-bltg t-bltg marked this pull request as draft January 12, 2023 14:39
@t-bltg t-bltg marked this pull request as ready for review January 12, 2023 14:51
@t-bltg t-bltg changed the title Fix silent LaTexString conversion Fix silent LaTeXString conversion Jan 12, 2023
@t-bltg t-bltg changed the title Fix silent LaTeXString conversion fix silent LaTeXString conversion Jan 12, 2023
Copy link
Collaborator

@ffreyer ffreyer left a comment

Choose a reason for hiding this comment

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

The test failure is just a communication error so this should be good to go right?

Error: Resource not accessible by integration

@t-bltg
Copy link
Collaborator Author

t-bltg commented Jan 17, 2023

No, this needs new reference images merged (label type change test), expanding Run mshick/add-pr-comment@v1 yields:

Run mshick/add-pr-comment@v1
  with:
    message: ## Missing reference images
  Found 1 new images without existing references.
  Upload new reference images before merging this PR.

    repo-token: ***
    allow-repeats: true
  env:
    MODERNGL_DEBUGGING: true
    ARTIFACTS_PATH: ~/.julia/artifacts
    PACKAGES_PATH: ~/.julia/packages
    REGISTRIES_PATH: 
    PRECOMPILATION_CACHE_PATH: 
    TESTS_SUCCESSFUL: true
Error: Resource not accessible by integration

@asinghvi17
Copy link
Member

I think there are a couple of new reference tests in this round of PRs, Simon mentioned he usually uploads those manually.

@t-bltg
Copy link
Collaborator Author

t-bltg commented Jan 17, 2023

IIUC, this can only be done by contributors with write access (which I don't have).

@asinghvi17
Copy link
Member

True - I meant that AFAIK Simon himself uploads those manually, after merging a bunch of PRs. CC @SimonDanisch - is this good to merge?

@jkrumbiegel jkrumbiegel merged commit b1b9ef2 into MakieOrg:master Feb 7, 2023
@t-bltg t-bltg deleted the trigger branch February 7, 2023 12:58
@github-actions
Copy link
Contributor

github-actions bot commented Feb 7, 2023

Missing reference images

Found 1 new images without existing references.
Upload new reference images before merging this PR.

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.

Axis3 axis label modifications with LaTeX do not get rendered (works for Axis)
4 participants