Skip to content

Conversation

@aihao2000
Copy link
Contributor

@aihao2000 aihao2000 commented Feb 23, 2024

What does this PR do?

Fixes # (issue)

Before submitting

Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

@sayakpaul and @patrickvonplaten

@sayakpaul
Copy link
Member

Thanks for your contributions! Can we please break this PR into multiple PRs as it targets multiple mutually exclusive areas?

@aihao2000
Copy link
Contributor Author

@sayakpaul Yes, feel free to change it.

@sayakpaul
Copy link
Member

Sorry for not being clear. I was asking you to tackle this in multiple PRs

@aihao2000
Copy link
Contributor Author

@sayakpaul I'm not familiar with git and it seems a little troublesome. Are there any doubtful changes? I can restore that part.

@sayakpaul
Copy link
Member

It's a general good practice to not introduce multiple changes in a single PR. And I would like to follow that. For example, for each of the points you enlisted in the PR description, you could create PRs (four PRs).

@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the contributing guidelines are likely to be ignored.

@github-actions github-actions bot added the stale Issues that haven't received updates label Mar 24, 2024
@sayakpaul
Copy link
Member

@aihao2000 a gentle ping here

@github-actions github-actions bot removed the stale Issues that haven't received updates label Mar 25, 2024
@aihao2000 aihao2000 force-pushed the update-train-scripts branch from 3432404 to 77697d4 Compare March 29, 2024 08:18
@aihao2000
Copy link
Contributor Author

@sayakpaul IIt is now only about the addition of the data_dir parameter. Is this okay?

@sayakpaul sayakpaul requested a review from lhoestq March 29, 2024 10:43
@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

Copy link
Member

@lhoestq lhoestq left a comment

Choose a reason for hiding this comment

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

Cool :) ust one comment:

Comment on lines 477 to 483
if args.dataset_name is None and args.train_data_dir is None:
raise ValueError("Need either a dataset name or a training folder.")
Copy link
Member

Choose a reason for hiding this comment

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

maybe keep this one ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lhoestq oh my bad

@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the contributing guidelines are likely to be ignored.

@github-actions github-actions bot added the stale Issues that haven't received updates label Apr 22, 2024
@yiyixuxu yiyixuxu removed the stale Issues that haven't received updates label Apr 22, 2024
@yiyixuxu
Copy link
Collaborator

@aihao2000 can we address the last comment and get this merged?

@aihao2000
Copy link
Contributor Author

@yiyixuxu Should I submit a new commit to restore this code?

@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the contributing guidelines are likely to be ignored.

@github-actions github-actions bot added the stale Issues that haven't received updates label Sep 14, 2024
@yiyixuxu
Copy link
Collaborator

@aihao2000 yes

@github-actions github-actions bot removed the stale Issues that haven't received updates label Sep 18, 2024
@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the contributing guidelines are likely to be ignored.

@github-actions github-actions bot added the stale Issues that haven't received updates label Oct 12, 2024
@yiyixuxu
Copy link
Collaborator

yiyixuxu commented Dec 3, 2024

can you run make style?

@yiyixuxu yiyixuxu added close-to-merge and removed stale Issues that haven't received updates labels Dec 3, 2024
@yiyixuxu yiyixuxu merged commit b58f67f into huggingface:main Dec 3, 2024
8 checks passed
@yiyixuxu
Copy link
Collaborator

yiyixuxu commented Dec 3, 2024

thanks @hlky !

lawrence-cj pushed a commit to lawrence-cj/diffusers that referenced this pull request Dec 4, 2024
* add data_dir parameter to load_dataset

---------

Co-authored-by: Sayak Paul <[email protected]>
Co-authored-by: YiYi Xu <[email protected]>
Co-authored-by: hlky <[email protected]>
sayakpaul added a commit that referenced this pull request Dec 23, 2024
* add data_dir parameter to load_dataset

---------

Co-authored-by: Sayak Paul <[email protected]>
Co-authored-by: YiYi Xu <[email protected]>
Co-authored-by: hlky <[email protected]>
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.

6 participants