Skip to content

Improve setup.py and add dependency check #5826

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Nov 17, 2023
Merged

Improve setup.py and add dependency check #5826

merged 5 commits into from
Nov 17, 2023

Conversation

patrickvonplaten
Copy link
Contributor

@patrickvonplaten patrickvonplaten commented Nov 16, 2023 •
edited
Loading

What does this PR do?

This PR makes sure that when PEFT is installed but it doesn't match what's written in the setup.py it fails gracefully.

The following will fail:

!pip install git+https://github.com/huggingface/peft.git
!pip install diffusers

and

import diffusers

with:

ImportError: peft<=0.6.2 is required for a normal functioning of this module, but found peft==0.6.3.dev0.

@patrickvonplaten patrickvonplaten changed the title Improve setup peft Improve setup.py and add dependency check Nov 16, 2023
patrickvonplaten
patrickvonplaten commented Nov 16, 2023
View reviewed changes
src/diffusers/utils/versions.py
@@ -0,0 +1,117 @@
# Copyright 2020 The HuggingFace Team. All rights reserved.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copied from transformers

@BenjaminBossan
Copy link
Member

Thanks for adding the check. There probably needs to be an option to toggle this check off so that we can run CI on main PEFT and discover potential issues early. Perhaps via some environment variable, could be "private" and only for our testing.

@sayakpaul
Copy link
Member

There probably needs to be an option to toggle this check off so that we can run CI on main PEFT and discover potential issues early. Perhaps via some environment variable, could be "private" and only for our testing.

Tend to agree with this. Otherwise, we won't be able to catch issues. What's the plan for that, @patrickvonplaten?

sayakpaul
sayakpaul reviewed Nov 17, 2023
View reviewed changes
src/diffusers/utils/versions.py
}


def _compare_versions(op, got_ver, want_ver, requirement, pkg, hint):
Copy link
Member

Choose a reason for hiding this comment

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

Nice. Then we should totally use it remove functions like compare_torch_versions().

@patrickvonplaten
Copy link
Contributor Author

Thanks for adding the check. There probably needs to be an option to toggle this check off so that we can run CI on main PEFT and discover potential issues early. Perhaps via some environment variable, could be "private" and only for our testing.

Good point!

@patrickvonplaten
Copy link
Contributor Author

patrickvonplaten commented Nov 17, 2023 •
edited
Loading

New functionality if PEFT > 0.6.2 is installed:

python -c "import diffusers"

will fail

_CHECK_PEFT=0 python -c "import diffusers"

won't fail

@sayakpaul
Copy link
Member

That works for me!

@BenjaminBossan
Copy link
Member

Thanks for adding the switch. The PEFT GH workflow requires it to be set to avoid failing the version check.

@patrickvonplaten
Copy link
Contributor Author

Thanks for adding the switch. The PEFT GH workflow requires it to be set to avoid failing the version check.

Will update this in #5838 !

@patrickvonplaten patrickvonplaten merged commit 913986a into main Nov 17, 2023
@kashif kashif deleted the improve_setup_peft branch December 5, 2023 08:59
yoonseokjin pushed a commit to yoonseokjin/diffusers that referenced this pull request Dec 25, 2023
@patrickvonplaten
Improve setup.py and add dependency check (huggingface#5826)
3d8ae97 
* put peft in requirements

* correct peft

* correct installs

* make style

* make style
AmericanPresidentJimmyCarter pushed a commit to AmericanPresidentJimmyCarter/diffusers that referenced this pull request Apr 26, 2024
@patrickvonplaten
Improve setup.py and add dependency check (huggingface#5826)
511dea3 
* put peft in requirements

* correct peft

* correct installs

* make style

* make style
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sayakpaul sayakpaul sayakpaul left review comments

@BenjaminBossan BenjaminBossan Awaiting requested review from BenjaminBossan

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@patrickvonplaten @BenjaminBossan @sayakpaul