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 for https://github.com/bigscience-workshop/promptsource/issues/20 #21

Merged

Conversation

Urmish
Copy link
Contributor

@Urmish Urmish commented May 23, 2021

  1. Changes the template.jinja before its saved
  2. Changes the keys in examples after its loaded

@stephenbach
Copy link
Member

Great, thanks!

A couple of thoughts:

  1. We should also update the dataset schema with the same cleaning. Maybe we can make a function and just call it on both dictionaries?

  2. I'm leaning towards not doing the replacement in line 242, because it might affect other things besides the variable names. For example, someone might want to use a dash in the text part of their prompt. If we change the schema and example, that should be sufficient.

What do you think?

@Urmish
Copy link
Contributor Author

Urmish commented May 25, 2021

Creating a utility function and (2) makes sense. Can you expand a bit on the dataset schema part. I am not sure if I follow which dictionary you are referring to.

@stephenbach
Copy link
Member

Hi,
I'm referring to the thing in the sidebar above "Select Example" that's called "Dataset Schema". It's rendered with the line

st.sidebar.write(render_features(dataset.features))

Thanks!

@Urmish
Copy link
Contributor Author

Urmish commented May 26, 2021

I see. Let me check how to do that. A quick way dint work as I am not sure if the dataset.features field is mutable. Running into
"AttributeError: Can't set attribute"

@Urmish
Copy link
Contributor Author

Urmish commented May 26, 2021

@stephenbach - Made the requested changes. But that will still not solve the problem as the text we enter for the template -
"{{text}}

Is this asking about a description, an entity, an abbreviation, a person, a quantity, or a location?
|||
{{ ["Description", "Entity", "Abbreviation", "Person", "Quantity", "Location"] [label-coarse] }}"

Has label-coarse and does lead to the error when the template is applied -

File "/Users/urmisht/Documents/bigScience/promptsource/promptsource/promptsource.py", line 294, in <module>
    prompt = template.apply(example)

So the replace statement for template.jinja will still be needed to render it correctly

@stephenbach
Copy link
Member

I was thinking that in the template it would be written

{{ ["Description", "Entity", "Abbreviation", "Person", "Quantity", "Location"] [label_coarse] }}

We'll have to say in the template writing guide that you can't use dashes as key names in Jinja, but since the features are displayed with '_', users can just code against the displayed examples.

@Urmish
Copy link
Contributor Author

Urmish commented May 27, 2021

I see. Thanks for the clarification. Then this branch has the required fixes. Do you want to give it a try @stephenbach ?

@stephenbach
Copy link
Member

Right, then I think it's good to go. I will merge it in, thanks!

@stephenbach
Copy link
Member

@Urmish looks like it's failing code quality checks. Can you add a commit running

make style
make quality

@Urmish
Copy link
Contributor Author

Urmish commented May 31, 2021

@stephenbach - Done

@Urmish
Copy link
Contributor Author

Urmish commented May 31, 2021

Sorry, I dint realize we had code quality checks setup until you mentioned it :)

@stephenbach
Copy link
Member

Having some weird issue with black (#39). Will just merge in for now.

@stephenbach stephenbach merged commit 4acbe5c into bigscience-workshop:main May 31, 2021
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.

2 participants