Tags objects do not support equality checking correctly

[stdedos]

I tried to unit-test a listener manipulating tags here: pytest-dev/pytest#5618 (comment) It seems that compares robot.model.Tags is kinda tricky. Is this wanted?

[pekkaklarck]

I can reproduce this:

>>> from robot.model import Tags
>>> Tags(['x']) == Tags(['x'])
False

This definitely isn't correct, but it luckily ought to be pretty easy to add __eq__ (and __ne__) to Tags. PR would be appreciated, but because I consider this a bug, I can fix it myself as well after my holidays and once the newparser branch is merged to master.

[stdedos]

My only objection is that, I don't feel confident enough doing so.

Regarless, here(^^) is a PR to consider.

[pekkaklarck]

One thing to take into account is that tags in Robot are case-, space- and underscore-insensitive. In other words, Tags(['X X']) == Tags(['x_x']) should be true. Also the order of tags shouldn't matter so Tags(['a', 'b']) == Tags(['b', 'a']) should be true as well.

An other thing to consider is that should Tags get more list methods. I added missing list methods to ItemList few days ago (#3261) and I guess same could be done to Tags as well. In this case the easiest solution would be using collections.abc.MutableSequence. I'm not sure is that worth the effort, though.

[stdedos]

One thing to take into account is that tags in Robot are case-, space- and underscore-insensitive. In other words, Tags(['X X']) == Tags(['x_x']) should be true. Also the order of tags shouldn't matter so Tags(['a', 'b']) == Tags(['b', 'a']) should be true as well.

Would it then be acceptable, only for comparison purposes, to normalize everything? (this.lower().replace('_', ' ') == otherthis.lower().replace('_', ' ') and sorted(this) == sorted(other))

Or should that be done during init (as it could lower operations per call)?

An other thing to consider is that should Tags get more list methods. I added missing list methods to ItemList few days ago (#3261) and I guess same could be done to Tags as well. In this case the easiest solution would be using collections.abc.MutableSequence. I'm not sure is that worth the effort, though.

It seems you have alpha-merged something. Should I retry the patchset? Should I try to implement the normalization above? Do nothing and close?

[pekkaklarck]

Normalization should be done when comparing. We want to preserver the original format so it cannot be done at __init__. I doubt normalization when needed really affects performance, especially when normally there aren't that many tags and comparisons ought to be pretty rare in general.