Show falsifying keyword arguments in the same order as the arguments

[asmeurer]

Falsifying examples are shown with positional arguments converted to keyword arguments. However, these are sorted in alphabetical order. This can be confusing. It would be better if the arguments were shown in the original order.

For example:

from hypothesis import given
from hypothesis.strategies import integers

@given(integers(), integers(), integers())
def test(b, a, c):
    assert a == b == c

gives

Falsifying example: test(
    a=0, b=0, c=1,
)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<stdin>", line 5, in test
  File "/Users/aaronmeurer/anaconda3/envs/ndindex/lib/python3.8/site-packages/hypothesis/core.py", line 1171, in wrapped_test
    raise the_error_hypothesis_found
  File "<stdin>", line 6, in test
AssertionError

I dug into it. Hypothesis internally converts the entire argument spec into a single strategy, and converts positional arguments into keyword arguments where possible.

The issue seems to be that fixed_dictionaries does not maintain key order for normal dictionaries. I'm not sure why this is the case, since dictionaries are supposed to be ordered since Python 3.6. Supposing that isn't going to change, however, I think this can be fixed by explicitly passing an OrderedDict

If this weren't the case, I believe this would just work as expected, except there is also an oddity where the keyword arguments are actually reversed for some reason here. I'm not entirely clear about that logic, but I guess it is supposed to handle the case where one argument to the zip is longer than the other (if so, it can easily be fixed by using an additional reversed on the zip).

In short, this patch fixes the issue

diff --git a/hypothesis-python/src/hypothesis/core.py b/hypothesis-python/src/hypothesis/core.py
index 6089b3efb..606c5d1e2 100644
--- a/hypothesis-python/src/hypothesis/core.py
+++ b/hypothesis-python/src/hypothesis/core.py
@@ -27,7 +27,7 @@ import traceback
 import types
 import warnings
 import zlib
-from collections import defaultdict
+from collections import defaultdict, OrderedDict
 from inspect import getfullargspec
 from io import StringIO
 from random import Random
@@ -436,7 +436,7 @@ def process_arguments_to_given(wrapped_test, arguments, kwargs, given_kwargs, ar
     search_strategy = TupleStrategy(
         (
             st.just(arguments),
-            st.fixed_dictionaries(given_kwargs).map(lambda args: dict(args, **kwargs)),
+            st.fixed_dictionaries(OrderedDict(given_kwargs)).map(lambda args: dict(args, **kwargs)),
         )
     )
 
@@ -971,9 +971,9 @@ def given(
         # positional arguments into keyword arguments for simplicity.
         if given_arguments:
             assert not given_kwargs
-            for name, strategy in zip(
+            for name, strategy in reversed(list(zip(
                 reversed(original_argspec.args), reversed(given_arguments)
-            ):
+            ))):
                 given_kwargs[name] = strategy
         # These have been converted, so delete them to prevent accidental use.
         del given_arguments

Although (to me at least), making fixed_dictionaries not sort normal dicts would be preferable.

[Zac-HD]

I'd be happy to accept a patch the patch above, as well as stopping st.fixed_dictionaries() from sorting the keys:

hypothesis/hypothesis-python/src/hypothesis/strategies/_internal/collections.py

Lines 233 to 239 in 3d5761e

	if isinstance(strategy_dict, OrderedDict):
	self.keys = tuple(strategy_dict.keys())
	else:
	try:
	self.keys = tuple(sorted(strategy_dict.keys()))
	except TypeError:
	self.keys = tuple(sorted(strategy_dict.keys(), key=repr))

IMO this made sense when we supported earlier versions of Python for consistency across versions, as the database requires a consistent order of keys, but as definition order is reliably preserved on Python 3.6+ we can unconditionally self.keys = tuple(strategy_dict.keys()).

[asmeurer]

OK. Should these be separate pull requests?

[Zac-HD]

I think combining them makes sense; with RELEASE_TYPE: minor in the change notes.

[asmeurer]

Once again procrastination proves to be a winning strategy for me.