Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dict comparisons insensitive to order #1788

Closed
MVrachev opened this issue Jan 21, 2022 · 2 comments
Closed

Dict comparisons insensitive to order #1788

MVrachev opened this issue Jan 21, 2022 · 2 comments
Assignees
Labels
backlog Issues to address with priority for current development goals discussion Discussions related to the design, implementation and operation of the project
Milestone

Comments

@MVrachev
Copy link
Collaborator

Description of issue or feature request:
In #1783 we dropped support for Python 3.6 and inside this pr, we decided to drop OrderedDicts as well: e3b267e.
The argument was that inside python 3.7+ dictionaries insertion order is preserved: https://docs.python.org/3/whatsnew/3.7.html.

Even though the insertion order is preserved we should keep in mind that there are still differences between OrderedDicts and regular dictionaries like when comparing them:

>>> OrderedDict([(1,1), (2,2)]) == OrderedDict([(2,2), (1,1)]) 
False
>>> dict([(1,1), (2,2)]) == dict([(2,2), (1,1)]) 
True

The question is:
Is it a problem if we have two dictionaries with data in a different order that are not the same?

If yes we have two solutions:

  • revert e3b267e
  • inside the __eq__ implementations make sure this will happen:
>>> list(dict([(1,1), (2,2)]).keys())  == list(dict([(2,2), (1,1)]).keys())
False

or in other words where we are using dictionaries make sure when comparing them that the order makes a difference.

@MVrachev MVrachev added the discussion Discussions related to the design, implementation and operation of the project label Jan 21, 2022
@MVrachev
Copy link
Collaborator Author

It's good to add solve this issue with #1775.

@jku jku added the backlog Issues to address with priority for current development goals label Jan 26, 2022
@MVrachev MVrachev self-assigned this Feb 7, 2022
@MVrachev MVrachev added this to the sprint 17 milestone Feb 9, 2022
@ivanayov ivanayov modified the milestones: sprint 17, sprint 18 Feb 23, 2022
@MVrachev
Copy link
Collaborator Author

I have implemented __eq__ for all classes inside tuf/api/metadata.py and I have added a separate commit making sure order is considered during comparison where it makes sense: 6ea5372.
That's why I think this issue is resolved.
If there is something to be added, feel free to reopen it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog Issues to address with priority for current development goals discussion Discussions related to the design, implementation and operation of the project
Projects
None yet
Development

No branches or pull requests

3 participants