[OrderedDict] Implement OrderedDict.move_to_end(key, last=False)
#155152
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/155152
Note: Links to docs will display an error until the docs builds have been completed. ⏳ 13 Pending, 4 Unrelated FailuresAs of commit 54feebe with merge base e619c6b ( FLAKY - The following jobs failed but were likely due to flakiness present on trunk:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
OrderedDict.move_to_end(key, last=False)OrderedDict.move_to_end(key, last=False)
torch/_dynamo/variables/dicts.py
Outdated
| dict_cls = dict if user_cls is collections.defaultdict else user_cls | ||
| self.items = dict_cls({make_hashable(x): v for x, v in items.items()}) |
There was a problem hiding this comment.
Do we support arbitrary dict subclasses? If so, this causes us to execute arbitrary user code in Dynamo, which is not what we want
There was a problem hiding this comment.
My understanding is that user_cls is the base dict class (dict, defaultdict, ordereddict) and not the user class. Maybe a better name should be more appropriate here.
There was a problem hiding this comment.
Oh no. You're right. This is actually the user class. Let me change it to be the dict base class.
| val = self.items[key] | ||
| self.items.pop(key) | ||
| self.items[key] = val |
There was a problem hiding this comment.
what was wrong with the old implementation?
There was a problem hiding this comment.
Oh, it's because of last=... keyword. It would be hard to implement move_to_end(key, last=False).
Stack from ghstack (oldest at bottom):
dict.update()with no args #158061fromkeysclassmethod #155608bool(OrderedDict)#155503hasattr(..., IteratorVariable)#155501__eq__for dict_items #155154OrderedDict.popitem(last=...)#155153OrderedDict.move_to_end(key, last=False)#155152set_fullgraph(False)intest_dict/test_ordered_dict#160156