nateraw / modelcards

📝 Utility to create, edit, and publish model cards on the Hugging Face Hub. [**Now lives in huggingface_hub**]
MIT License
15 stars 4 forks source link

Creating a card from a template string instead of file #61

Open BenjaminBossan opened 2 years ago

BenjaminBossan commented 2 years ago

I wanted to know if this would be something that could be added: Add a new class method from_string (or similar name) to initialize a ModelCard when the template is already stored as a string variable. That way, in some circumstances, we can avoid the "round-trip" through a file. Implementation-wise, it would be quite simple:

class ModelCard(RepoCard):
    @classmethod
    def from_template(
        cls,
        card_data: CardData,
        template_path: Optional[str] = TEMPLATE_MODELCARD_PATH,
        **template_kwargs,
    ):
        template_str = Path(template_path).read_text()
        return cls.from_string(card_data=card_data, template_str=template_str, **template_kwargs)

    @classmethod
    def from_string(
        cls,
        card_data: CardData,
        template_str: str,
        **template_kwargs,
    ):
        content = jinja2.Template(template_str).render(
            card_data=card_data.to_yaml(), **template_kwargs
        )
        return cls(content)
adrinjalali commented 2 years ago

instead of a new method, we could rename template_path to template, and either accept a file name or a file descriptor, and if the user already has the template in a string, they can pass it as a StringIO object.

BenjaminBossan commented 2 years ago

That would also be fine if we're okay with the possible backwards incompatibility caused by the renaming (instead, we could leave the parameter name as is, but that's a bit confusing).

adrinjalali commented 2 years ago

This hasn't been released really yet, so we could rename it I'd say. It's all happening here: https://github.com/huggingface/huggingface_hub/pull/940

osanseviero commented 2 years ago

Yes, feel free to open a PR against that PR as adding more features directly in the original PR would make the PR too large.

nateraw commented 2 years ago

Yea after using this tool for a while, this feature is definitely something that's come up as a need. I think you can either PR against my PR, or we can wait until mine is merged and I'll add this as an additional feature.

The PR is getting pretty big so I prefer the latter, but wouldn't complain if you did the former either. 🍻

Just depends on how urgently you'd like this 😉

nateraw commented 2 years ago

Though it is a pretty small addition so maybe it would be fine to add...

BenjaminBossan commented 2 years ago

There is no urgency, it would just be a convenient feature. The only reason to include it earlier rather than later is if we go with the proposal to rename the argument. Up to you to decide what you prefer.

nateraw commented 2 years ago

It's also pretty easy to do template string w/ f-strings, which might be a common use case...

from modelcards import ModelCard, CardData

data = CardData(license='mit', language='en', tags=['cardtest'])
content = f"""
---
{data.to_yaml()}
---

# My Model

blah blah blah
"""

card = ModelCard(content)

I'd probably encourage folks to use this unless they really needed Jinja, as we're going to have to have the Jinja dep be optional in huggingface_hub.