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.

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

convert : fix Norway problem when parsing YAML #12114

Merged
merged 3 commits into from
Feb 28, 2025

Conversation

ngxson
Copy link
Collaborator

@ngxson ngxson commented Feb 28, 2025

Related to #12091 (comment)

The problem is:

- no

The YAML above will be interpreted as "yes/no", which is translated into true/false boolean value.

The fix? Do a replace no --> "no"

CC @mofosyne @compilade if you have a better solution

@ngxson ngxson requested a review from ggerganov February 28, 2025 15:31
@github-actions github-actions bot added the python python script changes label Feb 28, 2025
@ngxson
Copy link
Collaborator Author

ngxson commented Feb 28, 2025

The problem seems to be fixed on yaml 1.2 specs, but pyyaml doesn't support 1.2: yaml/pyyaml#116

Comment on lines 146 to 149
yaml_content += "\n"
yaml_content = yaml_content.replace("- no\n", "- \"no\"\n")

if yaml_content:
Copy link
Collaborator

@compilade compilade Feb 28, 2025

Choose a reason for hiding this comment

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

Now this if is always true, because of the extra new line.

Maybe the extra \n should be added only when joining the lines?

diff --git a/gguf-py/gguf/metadata.py b/gguf-py/gguf/metadata.py
index 0629306bb..e807f4346 100644
--- a/gguf-py/gguf/metadata.py
+++ b/gguf-py/gguf/metadata.py
@@ -139,11 +139,10 @@ class Metadata:
                     break # End of frontmatter
                 else:
                     lines_yaml.append(line)
-            yaml_content = "\n".join(lines_yaml)
+            yaml_content = "\n".join(lines_yaml) + "\n"
 
         # Quick hack to fix the Norway problem
         # https://hitchdev.com/strictyaml/why/implicit-typing-removed/
-        yaml_content += "\n"
         yaml_content = yaml_content.replace("- no\n", "- \"no\"\n")
 
         if yaml_content:

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah yes thanks, implemented in bd40850

@ngxson ngxson merged commit 06c2b15 into ggml-org:master Feb 28, 2025
4 checks passed
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.

2 participants