-
Notifications
You must be signed in to change notification settings - Fork 7
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
Add end-to-end HuggingFace Example #758
base: language
Are you sure you want to change the base?
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.
Great work! this is some first review mostly about styling
key: str(value) if value is not None else "" | ||
for key, value in { | ||
"year": sample.get("YEAR"), | ||
"labels": sample.get("LABELS"), | ||
"meshes": sample.get("MESHES"), | ||
"long_answer": sample.get("LONG_ANSWER"), | ||
"reasoning_required": sample.get("reasoning_required_pred"), | ||
"reasoning_free": sample.get("reasoning_free_pred"), | ||
}.items() |
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.
We can keep it simple
return {
"year": sample.get("YEAR", ""),
"labels": sample.get("LABELS", ""),
...
}
or more complicated
return {
key.lower().removesuffix("_pred"): sample.get(key, "")
for key in ("YEAR", "LABELS", ...)
}
IMO simple is enough and more readable :)
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.
I tested it and if a key exists in the sample but the value is None, then sample.get(key, "")
will still return None, which is what I try to avoid. Maybe I'll use "year": sample.get("YEAR") or "",
? what do you think?
Add functionality to run HuggingFace models on text data.