-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
scripts: Use local gguf package when running from repo #2927
scripts: Use local gguf package when running from repo #2927
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a better workaround than symlinking
I'm gonna need a second opinion that I don't suck. |
Related: #2697 (comment) I had an issue previously, and I think this PR would've solved it (maybe even without my noticing). Thanks in general! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could the paths be made relative to __file__
so you can run the scripts from any directory, and so we don't have to check whether gguf-py exists?
Also, 'NO_LOCAL_GGUF' not in os.environ
is a more typical way of writing the env variable check.
Sounds reasonable. Is edit: # Use local gguf module if available.
if 'NO_LOCAL_GGUF' not in os.environ and (Path(__file__).parent.absolute().joinpath('gguf-py', 'gguf', '__init__.py')).is_file():
sys.path.insert(1, str(Path(__file__).parent.absolute().joinpath('gguf-py', 'gguf'))) |
I don't see why we're using In other words, I think we should just do this (with additional if 'NO_LOCAL_GGUF' not in os.environ:
sys.path.insert(1, str(Path(__file__).parent / 'gguf-py' / 'gguf')) |
Sure, that's no problem. I was just trying to be careful and avoid any potential weirdness, especially since I don't know exactly how stuff is going to work on Windows.
Just a habit I acquired (the hard way) of checking conditions only doing work if necessary. Makes it less likely for strange stuff to happen in the future. In this particular case it's very unlikely to cause a problem though. |
You can get the default behavior by setting the
NO_LOCAL_GGUF
environment variable.Personally I find the new behavior of ignoring the in-repo version of GGUF kind of unintuitive. This simple pull just adds the local
gguf
tosys.path
if it exists andNO_LOCAL_GGUF
is unset. That way one can use thegguf
version that's in sync with the scripts without having to worry about setting up environments or manually syncing.I didn't apply this to the convert-llama scripts because of #2906