Skip to content

gguf-py : GGUF Editor GUI - Python + Qt #12930

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

Merged
merged 26 commits into from
Apr 18, 2025

Conversation

christopherthompson81
Copy link
Contributor

I wrote a visual editor for GGUF files to be included in the gguf-py package. Hopefully, this fits in with the project.

@github-actions github-actions bot added the python python script changes label Apr 13, 2025
Copy link
Contributor Author

@christopherthompson81 christopherthompson81 left a comment

Choose a reason for hiding this comment

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

I've applied these. Thank-you!

@CISC
Copy link
Collaborator

CISC commented Apr 13, 2025

Looks great, only thing that concerns me is that it will be distributed with the gguf package, but not immediately usable since the requirements are not part of it.

I guess they could be part of an optional gui dependency f.ex. but not sure...

@christopherthompson81
Copy link
Contributor Author

Maybe it shouldn't be part of the gguf package, then? I just wasn't sure where else to put it. Let me know what I can do to help it along.

@CISC
Copy link
Collaborator

CISC commented Apr 13, 2025

Maybe it shouldn't be part of the gguf package, then? I just wasn't sure where else to put it. Let me know what I can do to help it along.

It makes sense to have it there, it's not that, but maybe getting some additional feedback on optional dependencies would be a good idea .. @compilade @mofosyne

@CISC
Copy link
Collaborator

CISC commented Apr 13, 2025

I'm getting this warning when opening a model BTW:

gguf_editor_gui.py:944: RuntimeWarning: Failed to disconnect (<bound method GGUFEditorWindow.on_metadata_changed of <__main__.GGUFEditorWindow(0x276d21b0) at 0x742c51b2e300>>) from signal "itemChanged(QTableWidgetItem*)".
  self.metadata_table.itemChanged.disconnect(self.on_metadata_changed)

@christopherthompson81
Copy link
Contributor Author

I've made a revision to filter warnings during loading because they're expected.

@CISC
Copy link
Collaborator

CISC commented Apr 15, 2025

Hey, sorry, I haven't forgotten you, just a little ill at the moment. :(

Everything looks good, but I would like you to look into two things first; adding the script and/or PySide6 as an optional gui install and documenting it in README.md.

Look at how the other scripts have been added to pyproject.toml and init.py and read up on poetry I guess. I will not be much help here, though I did ask Qwen which suggested adding:

[tool.poetry.group.gui.dependencies]
PySide6 = "^6.9"

@christopherthompson81
Copy link
Contributor Author

I've added the requested change. The poetry docs indicated that it should probably be an "extra".

@iwr-redmond
Copy link

iwr-redmond commented Apr 18, 2025

You may wish to consider using the qtpy compatibility layer rather than Pyside6 directly. Pyside6 v6.9 requires Python 3.10 or later, while gguf-py currently supports Python 3.8 or later. The qtpy package would allow Pyside2/QT5 support on older systems at the same time as allowing Pyside6/QT6 support on newer systems.

@CISC
Copy link
Collaborator

CISC commented Apr 18, 2025

I've added the requested change. The poetry docs indicated that it should probably be an "extra".

The docs say this is deprecated in favor of project.optional-dependencies, may not be an immediate issue though.

@CISC
Copy link
Collaborator

CISC commented Apr 18, 2025

You may wish to consider using the qtpy compatibility layer rather than Pyside6 directly. Pyside6 v6.9 requires Python 3.10 or later, while gguf-py currently supports Python 3.8 or later. The qtpy package would allow Pyside2/QT5 support on older systems at the same time as allowing Pyside6/QT6 support on newer systems.

As an optional dependency I don't think this is much of an issue, also TBH I'm not sure the >=3.8 is actually correct any more, the CI runs with 3.11 and I doubt anyone really checked.

@CISC
Copy link
Collaborator

CISC commented Apr 18, 2025

@christopherthompson81 Unless you want to rewrite this for qtpy I'm happy to merge this now, great work! :)

@christopherthompson81
Copy link
Contributor Author

Great! I think sticking with Pyside6 is fine. Go ahead with the merge.

@CISC CISC merged commit aff9d10 into ggml-org:master Apr 18, 2025
5 checks passed
@CISC
Copy link
Collaborator

CISC commented Apr 18, 2025

Sigh, build worked fine locally, not so much when publishing package:

The current project's supported Python range (>=3.8) is not compatible with some of the required packages Python requirement:
  - pyside6 requires Python <3.14,>=3.9, so it will not be installable for Python >=3.8,<3.9 || >=3.14

Because no versions of pyside6 match >6.9,<7.0
 and pyside6 (6.9.0) requires Python <3.14,>=3.9, pyside6 is forbidden.
So, because gguf depends on PySide6 (^6.9), version solving failed.

Guess I will have to figure if it really supports 3.8 now... :)

@iwr-redmond
Copy link

I reckon that the minimum viable change would be to use Pyside6>=6.6.3.1. That was the last release to support Python 3.8, and probably doesn't have too many, if any!, breaking changes versus Pyside6 v6.9.

@CISC
Copy link
Collaborator

CISC commented Apr 19, 2025

I reckon that the minimum viable change would be to use Pyside6>=6.6.3.1. That was the last release to support Python 3.8, and probably doesn't have too many, if any!, breaking changes versus Pyside6 v6.9.

Actually, it also complains about the <3.14 part, the proper fix seems to be adding these restrictions to the dependency options.

@bandoti
Copy link
Collaborator

bandoti commented Apr 19, 2025

@christopherthompson81 Thanks for implementing this!

Just a reminder when embarking on large efforts like this, it is important to maintain communication if there are outstanding issues or discussions that are related (like #6715), it can be helpful to inform them of a similar ongoing efforts.

That being said, please take a look at some discussions in #6715 and see if any of the features are relevant to this. Thank you.

@christopherthompson81
Copy link
Contributor Author

discussions in #6715 and see if any of the features are relevant to this.

They are! I wasn't aware of it. Thank-you for pointing it out.

I'm happy to keep working on the editor and while the initial PR is relatively basic, that's how these things usually go. They get more advanced over time. I'll see what I can do with the ideas over there.

@bandoti
Copy link
Collaborator

bandoti commented Apr 19, 2025

Sounds good. I will give the app a whirl and I would be happy to help expand on this as time permits. 😊

@sidkshatriya
Copy link

Would anyone mind sharing some screenshots of the GGUF editor on this PR -- it might give people a good idea of what the editor is about without having to necessarily install its dependencies and the editor first !

@christopherthompson81
Copy link
Contributor Author

Here are some screenshots:

The main editor and gemma-3-27b-it-qat-q4_0.gguf:
Screenshot_20250420_210307

Editing general.file_type:
Screenshot_20250420_211428
Screenshot_20250420_210503

Editing the tokenizer:
Screenshot_20250420_210559

Viewing the tensors:
Screenshot_20250420_211306

colout pushed a commit to colout/llama.cpp that referenced this pull request Apr 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python python script changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants