Skip to content

Conversation

ariG23498
Copy link
Contributor

No description provided.

@Wauplin
Copy link
Contributor

Wauplin commented Oct 15, 2025

I'm not a big fan of adding !pip install ... commands to the code snippets to be fair. It makes sense to have them in generated Colab notebooks so that "it just works" but snippets are usually meant to be minimal.

If one copy-paste the snippet on their machine or in a script, the !pip install ... line will fail (it works only in a notebook context)

Copy link
Member

@Vaibhavs10 Vaibhavs10 left a comment

Choose a reason for hiding this comment

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

Make them comments so they don't break.

This is a rare occasion when I disagree with @Wauplin - I think there's value in complete self contained snippets.


const diffusers_default = (model: ModelData) => [
`from diffusers import DiffusionPipeline
`!pip install -U transformers diffusers
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
`!pip install -U transformers diffusers
`# !pip install -U transformers diffusers


const diffusers_image_to_image = (model: ModelData) => [
`from diffusers import DiffusionPipeline
`!pip install -U transformers diffusers
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
`!pip install -U transformers diffusers
`# !pip install -U transformers diffusers


const diffusers_image_to_video = (model: ModelData) => [
`import torch
`!pip install -U transformers diffusers
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
`!pip install -U transformers diffusers
`# !pip install -U transformers diffusers


const diffusers_controlnet = (model: ModelData) => [
`from diffusers import ControlNetModel, StableDiffusionControlNetPipeline
`!pip install -U transformers diffusers
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
`!pip install -U transformers diffusers
`# !pip install -U transformers diffusers


const diffusers_lora = (model: ModelData) => [
`from diffusers import DiffusionPipeline
`!pip install -U transformers diffusers
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
`!pip install -U transformers diffusers
`# !pip install -U transformers diffusers


const diffusers_lora_text_to_video = (model: ModelData) => [
`from diffusers import DiffusionPipeline
`!pip install -U transformers diffusers
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
`!pip install -U transformers diffusers
`# !pip install -U transformers diffusers


const diffusers_lora_image_to_video = (model: ModelData) => [
`from diffusers import DiffusionPipeline
`!pip install -U transformers diffusers
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
`!pip install -U transformers diffusers
`# !pip install -U transformers diffusers


const diffusers_textual_inversion = (model: ModelData) => [
`from diffusers import DiffusionPipeline
`!pip install -U transformers diffusers
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
`!pip install -U transformers diffusers
`# !pip install -U transformers diffusers


const diffusers_flux_fill = (model: ModelData) => [
`import torch
`!pip install -U transformers diffusers
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
`!pip install -U transformers diffusers
`# !pip install -U transformers diffusers


const diffusers_inpainting = (model: ModelData) => [
`import torch
`!pip install -U transformers diffusers
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
`!pip install -U transformers diffusers
`# !pip install -U transformers diffusers

@ariG23498
Copy link
Contributor Author

@Wauplin were you suggesting the above changes? Apologies if I have made a mistake, I would love to know more.

Copy link
Contributor

@Wauplin Wauplin left a comment

Choose a reason for hiding this comment

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

@Wauplin were you suggesting the above changes? Apologies if I have made a mistake, I would love to know more.

Yes let's do this (as discussed in private slack)!

A nit would be to define a const diffusers_install string and use it only once here 😃

@ariG23498
Copy link
Contributor Author

Thanks for the suggestion @Wauplin !

I have updated accordingly. (I have not ever looked at TS, so if there are any core logical changes let me know)

Copy link
Contributor

@Wauplin Wauplin left a comment

Choose a reason for hiding this comment

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

Thanks for the update!

@Wauplin Wauplin merged commit e889326 into main Oct 17, 2025
5 checks passed
@Wauplin Wauplin deleted the ariG23498-patch-1 branch October 17, 2025 09:22
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.

3 participants