transformers icon indicating copy to clipboard operation
transformers copied to clipboard

Configurations should support id2label as a list

Open sijunhe opened this issue 2 years ago • 2 comments

Feature request

This is a rather minor/trivial feature request.

Currently id2label is of type Dict[int, str] in PretrainedConfig. Since this map is used to map class ids to labels, the keys is set to range(0, num_labels). Essentially, a list would also work here. I can even argue that a list is better in this case, because it avoids the potential error of the out-of-range keys and the weird serialization as

"id2label": {
    "0": "O",
    "1": "B-address",
    "2": "I-address",
    ...
}

Given that this attribute is mostly used for lookup id2label[i], it should be pretty easy to support this field both as a list and a map. We just need to fix a few places where we assumed .items() API exists.

I'll be happy to contribute a small PR for this.

Motivation

described above

Your contribution

Happy to contribute a PR for this.

sijunhe avatar Jul 07 '22 08:07 sijunhe

I would personally advocate for a single way of doing things, rather than many different ways of doing the same thing. In this situation, while the dict is more complex, it is also more complete.

LysandreJik avatar Jul 11 '22 10:07 LysandreJik

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the contributing guidelines are likely to be ignored.

github-actions[bot] avatar Aug 06 '22 15:08 github-actions[bot]