Skip to content

Validate each element in ClassLabel.int2str - #8484

Open
shashvat-singham wants to merge 1 commit into
huggingface:mainfrom
shashvat-singham:fix/classlabel-int2str-element-validation
Open

Validate each element in ClassLabel.int2str#8484
shashvat-singham wants to merge 1 commit into
huggingface:mainfrom
shashvat-singham:fix/classlabel-int2str-element-validation

Conversation

@shashvat-singham

Copy link
Copy Markdown

Partially fixes #8481

Problem

int2str validates the container it is given, but never the elements inside it. Three consequences, in decreasing order of severity:

>>> cl = ClassLabel(names=["neg", "pos"])

>>> cl.int2str([1.7])
['pos']                    # silently truncated by the int(v) used to index _int2str

>>> cl.int2str(["1"])
TypeError: '<=' not supported between instances of 'int' and 'str'

>>> cl.int2str([99.5])
ValueError: Unknown format code 'd' for object of type 'float'

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 TypeError case is the nested form of what #8416 fixes at the top level — that PR guards int2str("1"), this one covers int2str(["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:

cl.int2str([0, 1])                        # ['neg', 'pos']
cl.int2str(np.array([0, 1]))              # ['neg', 'pos']
cl.int2str([np.int64(0), np.int64(1)])    # ['neg', 'pos']

I deliberately checked integrality (int(v) != v) rather than isinstance(v, int), because iterating a numpy array or a torch/tf tensor yields scalar objects that are not Python ints — 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: _strval2int does str(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_elements covers the valid inputs (list, numpy array, numpy scalars) and each rejected case. It fails on main and passes with the change.

$ pytest tests/features/test_features.py -q
201 passed, 3 skipped

ruff format --check and ruff check are clean on both files.

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ClassLabel.str2int and int2str do not validate the elements of an iterable

1 participant