diffusers icon indicating copy to clipboard operation
diffusers copied to clipboard

The model_card_template is not included in the library

Open osanseviero opened this issue 3 years ago • 7 comments

Running the training colab fails when pushing to the Hub with

FileNotFoundError: [Errno 2] No such file or directory: '/usr/local/lib/python3.7/dist-packages/diffusers/utils/model_card_template.md'

Doing

!ls /usr/local/lib/python3.7/dist-packages/diffusers/utils/

gives

dummy_transformers_and_inflect_and_unidecode_objects.py  logging.py
dummy_transformers_objects.py				 __pycache__
__init__.py

~~Iirc, if you want a markdown file to be included you need to have include_package_data=True in the setup.py file.~~ Actually I see this is actually done :thinking:

cc @anton-l @nateraw

osanseviero avatar Jul 24 '22 14:07 osanseviero

This wasn't the case with a pip install from git, so something might be missing in our pypi pipeline (maybe MANIFEST.in isn't getting picked up?).

anton-l avatar Jul 24 '22 14:07 anton-l

I opened a PR :) https://github.com/huggingface/diffusers/pull/136

osanseviero avatar Jul 24 '22 14:07 osanseviero

@patrickvonplaten could you please check that the file is getting packaged when doing a release? Asking because of this: https://github.com/huggingface/diffusers/pull/136#issuecomment-1193327577

anton-l avatar Jul 24 '22 14:07 anton-l

Why do we work here with MANIFEST.in in general? Can't we just use a Python file to handle all model card specific logic like we do in transformers: https://github.com/huggingface/transformers/blob/main/src/transformers/modelcard.py .

IMO it would be easier for the comunity to just work with a simple Python file. What's the advantage of using the https://github.com/huggingface/diffusers/blob/main/src/diffusers/utils/model_card_template.md file where IMO people need to learn a new logic of how to replace Jinja2 placeholders and how to include .md files in the package?

Thoughts @anton-l @osanseviero @nateraw ?

BTW, the reason I didn't want https://github.com/nateraw/modelcards to be included in the required installs (see https://github.com/huggingface/diffusers/commit/416749ff9693d9aacbfd9d4b28443151caaa5e77) is that I would like to keep the package as slim as possible and because moodelcards depndens on Jinja2 on which I don't want to have a hard dependency on for a core ML library.

patrickvonplaten avatar Jul 24 '22 16:07 patrickvonplaten

@patrickvonplaten for context: @nateraw is already adding modelcards to hf_hub, so this was a POC for that integration: https://github.com/huggingface/huggingface_hub/pull/940

anton-l avatar Jul 25 '22 12:07 anton-l

Note that in hugginface_hub jinja will be an optional dependency, not a required install

osanseviero avatar Jul 25 '22 12:07 osanseviero

Answered in the thread internally - if we decide to go ahead with jinja and the huggingface_hub then happy to keep the same logic here until it's fully available in the hub

patrickvonplaten avatar Jul 25 '22 12:07 patrickvonplaten

Closing as this is no longer relevant

patrickvonplaten avatar Sep 13 '22 15:09 patrickvonplaten