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

ImmutableDict may fail to maintain iteration order on non-CPython implementations of Python 3.6 #52

Open
gabbard opened this issue Jun 10, 2019 · 6 comments
Labels
bug Something isn't working

Comments

@gabbard
Copy link

gabbard commented Jun 10, 2019

#51 made me notice our current code loses the ImmutableDict ordering guarantee on any 3.6 implementations which do not guarantee deterministic dict ordering in general because if we initialize from an Iterable of key-value pairs, we eventualy just pass the iterable directly to dict.

My inclination is to crash on module load in the same circumstances we currently crash when trying to initialize from a dict. That will prevent inadvertent bugs. If someone needs to run on a non-CPython 3.6 implementation, they can add the alternate implementation which maintains ordering. @ConstantineLignos ?

tag @berquist

@gabbard gabbard added the bug Something isn't working label Jun 10, 2019
@berquist
Copy link
Member

Is it as simple as changing

if isinstance(iterable, Dict) and not DICT_ITERATION_IS_DETERMINISTIC:
to

if not DICT_ITERATION_IS_DETERMINISTIC

? There should probably be a keyword argument to bypass this check (like for ImmutableSet).

@gabbard
Copy link
Author

gabbard commented Jun 10, 2019

@berquist : Aha, yes, you are correct (on both counts)

@ConstantineLignos
Copy link
Contributor

This all sounds good, just make sure that if that if is changed the error messages change accordingly.

@ConstantineLignos
Copy link
Contributor

Wait, maybe I'm missing something, isn't this entire package supposed to be 3.6+? From setup.py:

      # 3.6 and up, but not Python 4
      python_requires='~=3.6',

@ConstantineLignos
Copy link
Contributor

Sorry, I think was mixed up by the title. @rgabbard am I right that the issue is not about "Python < 3.6" as the title says, but only for non-CPython 3.6 where dicts are not guaranteed to have deterministic iteration order?

@gabbard gabbard changed the title ImmutableDict does not maintain iteration order on Python < 3.6 ImmutableDict may fail to maintain iteration order on non-CPython implementations of Python 3.6 Jun 12, 2019
@gabbard
Copy link
Author

gabbard commented Jun 12, 2019

@ConstantineLignos : Correct - apologies for the incorrect title

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants