Validate each element in ClassLabel.int2str - #8484
Open
shashvat-singham wants to merge 1 commit into
Open
Conversation
Partially fixes huggingface#8481 int2str validated the container it was given but never the elements inside it, so a non-integral element was silently truncated by the int(v) used to index _int2str: >>> cl = ClassLabel(names=["neg", "pos"]) >>> cl.int2str([1.7]) ['pos'] and a string element reached the range comparison and raised a cryptic TypeError rather than a useful message: >>> cl.int2str(["1"]) TypeError: '<=' not supported between instances of 'int' and 'str' An out-of-range non-integral value was worse still: it passed the comparison to the raise, where the "{v:d}" format itself failed with "Unknown format code 'd' for object of type 'float'". Validate each element instead. Integral values are still accepted from any type that converts exactly, so numpy integers and framework tensors keep working; only values that would have been truncated or compared against are rejected, with the offending value named.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Partially fixes #8481
Problem
int2strvalidates the container it is given, but never the elements inside it. Three consequences, in decreasing order of severity:The first is the one worth fixing: it returns a plausible-looking label for a value that was never a valid class id, with no error. The third is the existing
f"...{v:d}"raise failing on the very value it was trying to report.The
TypeErrorcase is the nested form of what #8416 fixes at the top level — that PR guardsint2str("1"), this one coversint2str(["1"]).Change
Validate each element in the loop that already walks them. A value is accepted if it converts to an integer exactly, so this keeps working for every input the docstring promises:
I deliberately checked integrality (
int(v) != v) rather thanisinstance(v, int), because iterating a numpy array or a torch/tf tensor yields scalar objects that are not Pythonints — a type check would have broken the framework-tensor inputs the docstring explicitly supports.Scope note
#8481 also reports
str2int([1])returning[1]. Having read the code I think that one is more defensible than I made it sound in the issue:_strval2intdoesstr(value)first and numeric strings like"1"are deliberately supported, so an integer falling through is a natural consequence of that feature rather than a separate bug. I've left it alone — happy to revisit if you read it differently.Tests
test_classlabel_int2str_validates_elementscovers the valid inputs (list, numpy array, numpy scalars) and each rejected case. It fails onmainand passes with the change.ruff format --checkandruff checkare clean on both files.