Skip to content
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

Update convert-hf-to-gguf.py #8015

Merged
merged 2 commits into from
Jun 22, 2024
Merged

Update convert-hf-to-gguf.py #8015

merged 2 commits into from
Jun 22, 2024

Conversation

0xspringtime
Copy link
Contributor

Replacing with a less risky alternative to the 'assert' statement

@0xspringtime
Update convert-hf-to-gguf.py
620633e 
Replacing with a less risky alternative to the 'assert' statement
@github-actions github-actions bot added the python python script changes label Jun 19, 2024
0xspringtime
0xspringtime commented Jun 19, 2024
View reviewed changes
Copy link
Contributor Author

@0xspringtime 0xspringtime left a comment

Choose a reason for hiding this comment

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

syntax correction

@mofosyne mofosyne added Review Complexity : Low Trivial changes to code that most beginner devs (or those who want a break) can tackle. e.g. UI fix merge ready indicates that this may be ready to merge soon and is just holding out in case of objections and removed merge ready indicates that this may be ready to merge soon and is just holding out in case of objections labels Jun 19, 2024
@Galunid Galunid merged commit 3aa184a into ggerganov:master Jun 22, 2024
17 of 19 checks passed
@HanClinto
Copy link
Collaborator

Too many blank lines were added on line 977-ish, so it's now breaking CI. Not a huge deal, but wanted to let you know. I will fix in another PR.

Make sure to double-check failing CI steps before merging. Sometimes build failures are fine (like the finicky Docker builds), but things like the Lint check should always pass. Always double-check failures to ensure that they're not caused by the PR prior to merging.

@HanClinto
Copy link
Collaborator

I'm also not sure how the CI treats Python-only changes, and it's possible that CI didn't run until merge, so it might not have been easy to see this one.

@HanClinto HanClinto mentioned this pull request Jun 22, 2024
Merged
4 tasks
@Galunid
Copy link
Collaborator

Galunid commented Jun 22, 2024

My bad, sorry about that and thanks for fixing!

@HanClinto
Copy link
Collaborator

My bad, sorry about that and thanks for fixing!

No worries! Looking at it now, I'm really not sure if the CI ran prior to the merge or not, so I don't know if there was a way you reasonably could have caught it.

We may need to modify the CI triggers on Python-only changes -- I'm really not sure.

Either way, not a big deal. I've made my share of merge hiccups, and it's just something to take in stride. :)

Thanks for being pro-active with reviews and merges!

arthw pushed a commit to arthw/llama.cpp that referenced this pull request Jun 30, 2024
@0xspringtime @NeoZhangJianyu
convert-hf : change assert to exception (ggerganov#8015)
9a6d2b7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Galunid Galunid Galunid approved these changes

Assignees
No one assigned
Labels
python python script changes Review Complexity : Low Trivial changes to code that most beginner devs (or those who want a break) can tackle. e.g. UI fix
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@0xspringtime @HanClinto @Galunid @mofosyne