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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds Latex Delimiters Parameter #4516

Merged
merged 13 commits into from Jun 15, 2023
Merged

Adds Latex Delimiters Parameter #4516

merged 13 commits into from Jun 15, 2023

Conversation

dawoodkhan82
Copy link
Collaborator

Description

Adds enable_latex and latex_delimiters parameters.

Please include:

  • relevant motivation
  • a summary of the change
  • which issue is fixed.
  • any additional dependencies that are required for this change.

Closes: #4428

Checklist:

  • I have performed a self-review of my own code
  • I have added a short summary of my change to the CHANGELOG.md
  • My code follows the style guidelines of this project
  • I have commented my code in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

A note about the CHANGELOG

Hello 馃憢 and thank you for contributing to Gradio!

All pull requests must update the change log located in CHANGELOG.md, unless the pull request is labeled with the "no-changelog-update" label.

Please add a brief summary of the change to the Upcoming Release > Full Changelog section of the CHANGELOG.md file and include
a link to the PR (formatted in markdown) and a link to your github profile (if you like). For example, "* Added a cool new feature by [@myusername](link-to-your-github-profile) in [PR 11111](https://github.com/gradio-app/gradio/pull/11111)".

If you would like to elaborate on your change further, feel free to include a longer explanation in the other sections.
If you would like an image/gif/video showcasing your feature, it may be best to edit the CHANGELOG file using the
GitHub web UI since that lets you upload files directly via drag-and-drop.

@dawoodkhan82
Copy link
Collaborator Author

@abidlabs lmk if you think adding both params is unnecessary. Maybe we could use passing None into latex_delimiters as a way to turn off latex rendering.

@gradio-pr-bot
Copy link
Contributor

All the demos for this PR have been deployed at https://huggingface.co/spaces/gradio-pr-deploys/pr-4516-all-demos

@abidlabs
Copy link
Member

Nice! Yeah, I think both is unnecessary. You could just pass in an empty list to latex_delimiters to indicate no LaTeX should be rendered. (We should add this to the docstring for latex_delimiters)

Also, we should mark this as a breaking change in the changelog as now by default we are only rendering LaTeX if enclosed with double $ signs.

@abidlabs
Copy link
Member

Some suggestions on the docs above. But tested and it works well @dawoodkhan82!

@dawoodkhan82
Copy link
Collaborator Author

Display false:
Screenshot 2023-06-14 at 3 14 01 PM

Display True:
Screenshot 2023-06-14 at 3 13 44 PM

Basically inline or not

@dawoodkhan82
Copy link
Collaborator Author

You could just pass in an empty list to latex_delimiters to indicate no LaTeX should be rendered. (We should add this to the docstring for latex_delimiters)

Should it also accept None so either empty list or None can disable latex.

@abidlabs
Copy link
Member

Should it also accept None so either empty list or None can disable latex.

Sure, sg

@abidlabs
Copy link
Member

@dawoodkhan82 this looks good. However, it's strongly recommended not to use lists (or other mutables) as argument defaults.

So I'm going to change this to set the default value of latex_delimiters to None, which internally will be set to [{"display": True, "left": "$$", "right": "$$"}]. This means that we'll have to reverse this change:

Should it also accept None so either empty list or None can disable latex.

So now if you want to disable LaTeX altogether, you need to pass in an empty list

Copy link
Member

@pngwn pngwn left a comment

Choose a reason for hiding this comment

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

Few nits, but lgtm!

Comment on lines 18 to 22
export let latex_delimiters: Array<{
left: string;
right: string;
display: boolean;
}> = [{ left: "$$", right: "$$", display: true }];
Copy link
Member

Choose a reason for hiding this comment

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

Since this is always set in the backend I don't think we need this default.

Comment on lines 22 to 26
export let latex_delimiters: Array<{
left: string;
right: string;
display: boolean;
}> = [{ left: "$$", right: "$$", display: true }];
Copy link
Member

Choose a reason for hiding this comment

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

Ditto for here.

@abidlabs
Copy link
Member

abidlabs commented Jun 15, 2023

Tested the latest version with:

import gradio as gr

latex = """
I want to eat $3.4 worth of bananas and $2 worth of apples. The equation for the sum is $$3.4+2=5.4$$ thanks!
"""

message = [(
    "write a python program",
    latex
), (
    "write a python program",
    latex
)]

with gr.Blocks() as demo:
    gr.Chatbot(
        message
    )
    
demo.launch()

And looks good to me @pngwn!

image

@pngwn
Copy link
Member

pngwn commented Jun 15, 2023

Awesome, thanks @abidlabs!

@pngwn pngwn merged commit 03edbc6 into main Jun 15, 2023
9 of 13 checks passed
@pngwn pngwn deleted the katex-delim branch June 15, 2023 13:27
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.

Dollar signs automatically render latex - can happen unintentionally
4 participants