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

[Task Label & Icon] for Image-to-Video #317

Merged
merged 5 commits into from
Nov 23, 2023
Merged

Conversation

mishig25
Copy link
Collaborator

@mishig25 mishig25 commented Nov 22, 2023

This is how it looks

image

@mishig25 mishig25 marked this pull request as ready for review November 22, 2023 09:56
@mishig25 mishig25 requested a review from coyotte508 November 22, 2023 09:56
@mishig25 mishig25 force-pushed the task_icon_img_to_video branch from 4b03ee0 to 6764b0e Compare November 22, 2023 09:57
@mishig25 mishig25 changed the title [Task & Icon] for Image-to_video [Task & Icon] for Image-to-Video Nov 22, 2023
export let classNames = "";
</script>

<svg xmlns="http://www.w3.org/2000/svg" class={classNames} width="1em" height="1em" fill="none"><path fill="currentColor" fill-rule="evenodd" d="M1.43 1.2h6.63a.7.7 0 0 1 .7.7v4.44a.79.79 0 0 0-.34-.08h-.36V1.9H1.43v3.55l.9-.9a.7.7 0 0 1 .99 0l1.74 1.74a.79.79 0 0 0-.51.47L2.82 5.04l-1.4 1.4v2.09H4.5v.7H1.43a.7.7 0 0 1-.7-.7V1.9a.7.7 0 0 1 .7-.7ZM6.8 5.94l.32.32H5.5l.32-.32a.7.7 0 0 1 .99 0ZM6.2 4.52a1.05 1.05 0 1 1-1.17-1.75A1.05 1.05 0 0 1 6.2 4.52Zm-.4-1.16a.35.35 0 1 0-.38.58.35.35 0 0 0 .39-.58Zm3.9 5.16 1.57-1.26v3.14L9.7 9.15V10a.79.79 0 0 1-.79.79H5.77a.79.79 0 0 1-.79-.79V7.65a.79.79 0 0 1 .79-.78H8.9a.79.79 0 0 1 .79.78v.87Zm-3.93-.87v2.36H8.9V7.65H5.77Z" clip-rule="evenodd"/></svg>
Copy link
Collaborator Author

@mishig25 mishig25 Nov 22, 2023

Choose a reason for hiding this comment

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

@coyotte508 I think the formatter from #297 is not configured properly?
Shouldn't it be multline like

<svg
class={classNames}
xmlns="http://www.w3.org/2000/svg"
xmlns:xlink="http://www.w3.org/1999/xlink"
aria-hidden="true"
fill="currentColor"
focusable="false"
role="img"
width="1em"
height="1em"
preserveAspectRatio="xMidYMid meet"
viewBox="0 0 32 32"
>
<path
d="M29.537 13.76l-3.297-3.297a1.586 1.586 0 0 0-2.24 0L10 24.467V30h5.533l14.004-14a1.586 1.586 0 0 0 0-2.24zM14.704 28H12v-2.704l9.44-9.441l2.705 2.704zM25.56 17.145l-2.704-2.704l2.267-2.267l2.704 2.704z"
fill="currentColor"
/><path d="M11 17h2v-7h3V8H8v2h3v7z" fill="currentColor" /><path
d="M8 20H4V4h16v4h2V4a2 2 0 0 0-2-2H4a2 2 0 0 0-2 2v16a2 2 0 0 0 2 2h4z"
fill="currentColor"
/>
</svg>
?

Copy link
Member

Choose a reason for hiding this comment

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

hm yes maybe there's an issue with svelte files

@mishig25 mishig25 changed the title [Task & Icon] for Image-to-Video [Task Label & Icon] for Image-to-Video Nov 22, 2023
Copy link
Contributor

@osanseviero osanseviero left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM!

@@ -18,6 +18,7 @@ export const TASKS_MODEL_LIBRARIES: Record<PipelineType, ModelLibraryKey[]> = {
"image-segmentation": ["transformers", "transformers.js"],
"image-to-image": [],
"image-to-text": ["transformers.js"],
"image-to-video": [], // TODO: diffusers will soon support image-to-video
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add it already, it will be ready this week

Suggested change
"image-to-video": [], // TODO: diffusers will soon support image-to-video
"image-to-video": ["diffusers"],

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@osanseviero shouldn't this line also be diffusers ?

"text-to-image": [],

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

wdyt e1d2b92 ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

for another PR, we should probably unify packages/tasks/src/const.ts & packages/widgets/src/lib/interfaces/LibrariesToTasks.ts. Updating both of them is error-prone

Copy link
Member

Choose a reason for hiding this comment

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

yep TBH i didn't even remember the tasks/src/const.ts one

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes @mishig25 but let's keep this PR for image-to-video? For text-to-image actually I have #320 being updated with the migration

Copy link
Contributor

Choose a reason for hiding this comment

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

They are different though. LibrariesToTasks defines which tasks are supported by the Inference API for the specific library. This file is automatically generated by a GitHub Actions workflow in api-inference-community. const.ts is about library support, not specific to Inference API

Copy link
Collaborator Author

@mishig25 mishig25 Nov 22, 2023

Choose a reason for hiding this comment

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

let's keep this PR for image-to-video? For text-to-image actually I have #320 being updated with the migration

handled in cb0f277 and 6764b0e

@@ -54,6 +54,7 @@ export const TASKS_DATA: Record<PipelineType, TaskData | undefined> = {
"image-segmentation": getData("image-segmentation", imageSegmentation),
"image-to-image": getData("image-to-image", imageToImage),
"image-to-text": getData("image-to-text", imageToText),
"image-to-video": undefined,
Copy link
Member

Choose a reason for hiding this comment

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

Just a question to help me understand how it works, would creating an entry in tasks/data and populating it here would be enough to have the task listed in https://huggingface.co/tasks ?

Copy link
Member

Choose a reason for hiding this comment

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

That's it, yes

for now the sync between this repo and the hub is'nt active (needs https://github.com/huggingface/moon-landing/pull/8158 + a follow-up PR)

but once it's active yes (there will be some stuff like publishing/updating packages but it can be automated by tooling)

Copy link
Member

Choose a reason for hiding this comment

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

Awesome, thanks!

@mishig25 mishig25 merged commit 5005353 into main Nov 23, 2023
1 check passed
@mishig25 mishig25 deleted the task_icon_img_to_video branch November 23, 2023 09:33
kohsheen1234 pushed a commit to kohsheen1234/huggingface.js that referenced this pull request Nov 24, 2023
Follow up to huggingface#297 &
huggingface#317 (comment)

1. Moved deps to devDeps (cc: @coyotte508 please check)
2. Prettier was not running on svelte files. Upgrading prettier fixed
the issue
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.

5 participants