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

Issue with recursive build strategy? #3026

Closed
mristin opened this issue Jul 6, 2021 · 9 comments · Fixed by #3164
Closed

Issue with recursive build strategy? #3026

mristin opened this issue Jul 6, 2021 · 9 comments · Fixed by #3164
Labels
bug something is clearly wrong here

Comments

@mristin
Copy link
Contributor

mristin commented Jul 6, 2021

Hi,
I observed that hypothesis.strategy.builds does not work on the recursive structures. Here is a short program, where SomeClass represents a data structure whose property is also of type SomeClass (e.g., a node in a linked list):

from typing import Optional

import hypothesis.strategies as st


class SomeClass:
    value: int
    next_node: Optional['SomeClass']

    def __init__(self, value: int, next_node: Optional['SomeClass']) -> None:
        assert value > 0
        self.value = value
        self.next_node = next_node


def main() -> None:
    """Execute the main routine."""

    build_strategy_for_some_class = st.fixed_dictionaries(
        {
            'value': st.integers(min_value=1),
            'next_node': st.one_of(st.none(), st.builds(SomeClass))
        }
    ).map(lambda kwargs: SomeClass(**kwargs))

    st.register_type_strategy(SomeClass, build_strategy_for_some_class)

    strategy = st.from_type(SomeClass)
    print(f"strategy is {strategy!r}")
    # strategy is fixed_dictionaries({'next_node': one_of(none(), builds(SomeClass)),
    #  'value': integers(min_value=1)}).map(lambda kwargs: <unknown>)

    example = strategy.example()
    # Traceback
    #     ...
    #     assert value > 0
    # AssertionError


if __name__ == "__main__":
    main()

While the strategy has been correctly registered (see the print statement), the builds strategy does not work as I'd expect it. The strategy builds infers the arguments for SomeClass directly from its __init__ instead of checking whether the type has been registered.

If I change:

'next_node': st.one_of(st.none(), st.builds(SomeClass))

to

'next_node': st.one_of(st.none(), st.from_type(SomeClass))

I get the same assertion error.

Is this a bug or just a misunderstanding on my part?

Some related issues and articles:

  • This issue is not related to Issues with recursive data classes #3016 , which concerns the type annotations.

  • The article https://hypothesis.works/articles/recursive-data/ talks about generating recursive built-in structures. We need to use a non-built-in class SomeClass here.

  • The article in the documentation https://hypothesis.readthedocs.io/en/latest/data.html#recursive-data talks about pitfalls when generating recursive data, but the issue here is that hypothesis.strategy.builds(.) disregard the arguments.

    In the current example, the focus is on the recursivity in terms of classes (a class has a property of the same class, not in terms of actual data structures). While any_of strategy might result in an endless linked list, it is actually not probable (the length of the linked list follows geometric distribution, if I am not mistaken).

(Edit: added proper syntax highlighting)

@mristin mristin closed this as completed Jul 6, 2021
@mristin
Copy link
Contributor Author

mristin commented Jul 6, 2021

(Please apologize, wrong click.)

@mristin mristin reopened this Jul 6, 2021
@mristin
Copy link
Contributor Author

mristin commented Jul 6, 2021

I dug a bit deeper; I didn't understand deferred correctly, and finally came up with a solution:

strategy = st.deferred(
	lambda:
	st.fixed_dictionaries(
		{
			'value': st.integers(min_value=1),
			'next_node': st.one_of(st.none(), strategy)
		}
	).map(lambda kwargs: SomeClass(**kwargs))
)

However, now I am pretty lost how to automate this behavior in icontract-hypothesis. Ideally, I'd like to do something like:

strategy = st.deferred(
	lambda:
	(
		st.register_type_strategy(strategy),
                #  Here I'd like to make use of the registered strategy
		icontract_hypothesis.infer_strategy(some_cls)
	)[1]
)

where icontract_hypothesis.infer_strategy uses the global type lookup (hypothesis.strategies._internal.types._global_type_lookup).

Unfortunately, this causes an endless recursion. Do you happen to have any ideas how this behavior can be achieved?

@Zac-HD
Copy link
Member

Zac-HD commented Jul 7, 2021

I think the trick you're looking for is st.deferred(lambda: st.from_type(SomeClass)), as in:

build_strategy_for_some_class = st.builds(
    SomeClass,
    value=st.integers(min_value=1),
    next_node=st.none() | st.deferred(lambda: st.from_type(SomeClass)),
)
st.register_type_strategy(SomeClass, build_strategy_for_some_class)

And it's safe (if a little less efficient) to wrap st.deferred(lambda: ...) around any way to construct a strategy, so long as you don't remove things from scope later.

@Zac-HD Zac-HD added the question not sure it's a bug? questions welcome label Jul 7, 2021
@mristin
Copy link
Contributor Author

mristin commented Jul 7, 2021

Hi @Zac-HD ,
Thanks a lot for the pointer! I'm really stuck here :-).

I tried the following solution:

# STEP 1
st.register_type_strategy(
	SomeClass,
	st.deferred(lambda: st.from_type(SomeClass)))

# STEP 2
strategy = st.builds(
	SomeClass,
	label=st.integers(min_value=1),
	next_node=st.one_of(
			st.none(), st.from_type(SomeClass))

st.register_type_strategy(
	SomeClass,
	strategy
)

# STEP 3
strategy_str = str(strategy)

This gives an endless recursion (at STEP 3). Here's a snippet from the traceback:

...
File "<site-packages>\hypothesis\strategies\_internal\lazy.py", line 162, in __repr__
    arg_string(self.function, _args, kwargs_for_repr, reorder=False),
  File "<site-packages>\hypothesis\internal\reflection.py", line 413, in arg_string
    bits.append(f"{a}={nicerepr(kwargs[a])}")
  File "<site-packages>\hypothesis\internal\reflection.py", line 397, in nicerepr
    return re.sub(r"(\[)~([A-Z][a-z]*\])", r"\g<1>\g<2>", pretty(v))
  File "<site-packages>\hypothesis\vendor\pretty.py", line 118, in pretty
    printer.pretty(obj)
  File "<site-packages>\hypothesis\vendor\pretty.py", line 387, in pretty
    return _default_pprint(obj, self, cycle)
  File "<site-packages>\hypothesis\vendor\pretty.py", line 499, in _default_pprint
    _repr_pprint(obj, p, cycle)
  File "<site-packages>\hypothesis\vendor\pretty.py", line 727, in _repr_pprint
    output = repr(obj)
  File "<site-packages>\hypothesis\strategies\_internal\strategies.py", line 653, in __repr__
    return "one_of(%s)" % ", ".join(map(repr, self.original_strategies))
  File "<site-packages>\hypothesis\strategies\_internal\deferred.py", line 81, in __repr__
    return repr(self.__wrapped_strategy)
  File "<site-packages>\hypothesis\strategies\_internal\lazy.py", line 162, in __repr__
    arg_string(self.function, _args, kwargs_for_repr, reorder=False),
  File "<site-packages>\hypothesis\internal\reflection.py", line 413, in arg_string
    bits.append(f"{a}={nicerepr(kwargs[a])}")
  File "<site-packages>\hypothesis\internal\reflection.py", line 397, in nicerepr
    return re.sub(r"(\[)~([A-Z][a-z]*\])", r"\g<1>\g<2>", pretty(v))
  File "<site-packages>\hypothesis\vendor\pretty.py", line 118, in pretty
    printer.pretty(obj)
  File "<site-packages>\hypothesis\vendor\pretty.py", line 387, in pretty
    return _default_pprint(obj, self, cycle)
  File "<site-packages>\hypothesis\vendor\pretty.py", line 499, in _default_pprint
    _repr_pprint(obj, p, cycle)
  File "<site-packages>\hypothesis\vendor\pretty.py", line 727, in _repr_pprint
    output = repr(obj)
  File "<site-packages>\hypothesis\strategies\_internal\strategies.py", line 653, in __repr__
    return "one_of(%s)" % ", ".join(map(repr, self.original_strategies))
  File "<site-packages>\hypothesis\strategies\_internal\strategies.py", line 653, in __repr__
    return "one_of(%s)" % ", ".join(map(repr, self.original_strategies))
  File "<site-packages>\hypothesis\strategies\_internal\lazy.py", line 162, in __repr__
    arg_string(self.function, _args, kwargs_for_repr, reorder=False),
  File "<site-packages>\hypothesis\internal\reflection.py", line 413, in arg_string
    bits.append(f"{a}={nicerepr(kwargs[a])}")
  File "<site-packages>\hypothesis\internal\reflection.py", line 397, in nicerepr
    return re.sub(r"(\[)~([A-Z][a-z]*\])", r"\g<1>\g<2>", pretty(v))
  File "<site-packages>\hypothesis\vendor\pretty.py", line 118, in pretty
    printer.pretty(obj)
  File "<site-packages>\hypothesis\vendor\pretty.py", line 387, in pretty
    return _default_pprint(obj, self, cycle)
  File "<site-packages>\hypothesis\vendor\pretty.py", line 499, in _default_pprint
    _repr_pprint(obj, p, cycle)
  File "<site-packages>\hypothesis\vendor\pretty.py", line 727, in _repr_pprint
    output = repr(obj)
  File "<site-packages>\hypothesis\strategies\_internal\strategies.py", line 653, in __repr__
    return "one_of(%s)" % ", ".join(map(repr, self.original_strategies))
  File "<site-packages>\hypothesis\strategies\_internal\strategies.py", line 653, in __repr__
    return "one_of(%s)" % ", ".join(map(repr, self.original_strategies))
  File "<site-packages>\hypothesis\strategies\_internal\lazy.py", line 162, in __repr__
    arg_string(self.function, _args, kwargs_for_repr, reorder=False),
  File "<site-packages>\hypothesis\internal\reflection.py", line 413, in arg_string
    bits.append(f"{a}={nicerepr(kwargs[a])}")
  File "<site-packages>\hypothesis\internal\reflection.py", line 397, in nicerepr
    return re.sub(r"(\[)~([A-Z][a-z]*\])", r"\g<1>\g<2>", pretty(v))
  File "<site-packages>\hypothesis\vendor\pretty.py", line 118, in pretty
    printer.pretty(obj)
  File "<site-packages>\hypothesis\vendor\pretty.py", line 387, in pretty
    return _default_pprint(obj, self, cycle)
...

Thank you very much for any pointers!

@mristin
Copy link
Contributor Author

mristin commented Jul 7, 2021

Please let me also add some more context why I register the strategy twice (step 1 and step 2) as this might have been lost.

The strategies are constructed programmatically. In that use case, I rely on Hypothesis and its from_type to figure out the strategies.

For example, st.from_type(Optional[SomeClass]) gives st.one_of(st.none(), st.from_type(SomeClass)). Unless I monkey-patch from_type, I can not really change the behavior of how the Optional[SomeClass] is interpreted into a strategy.

@mristin
Copy link
Contributor Author

mristin commented Jul 7, 2021

I also tried playing with placeholders in the lambda.

As far as I understand, Python binds the variables in lambda by reference; consider the following snippet:

x = 1
some_lambda = lambda: x
print(f"some_lambda() is {some_lambda()!r}")
# some_lambda() is 1

x = 2
print(f"some_lambda() is {some_lambda()!r}")
# some_lambda() is 2

However, when I try the same "trick" with the strategies, I get unexpected result:

placeholder = st.just(1977)
strategy = st.deferred(lambda: placeholder)
print(f"Step 1: strategy is {strategy!r}")
# Step 1: strategy is deferred(lambda: placeholder)

st.register_type_strategy(
	SomeClass,
	strategy)

strategy = st.from_type(SomeClass)
print(f"Step 2: strategy is {strategy!r}")
# Step 2: strategy is just(1977)

placeholder = st.just(2021)
strategy = st.from_type(SomeClass)
print(f"Step 3: strategy is {strategy!r}")
# Step 3: strategy is just(1977) <<< UNEXPECTED

(I marked the unexpected spot with UNEXPECTED.) I haven't drawn from the strategy, so st.deferred should not be actually evaluated, should it? But in this example, it seems that the deferred is actually evaluated and not deferred any more.

I tried to figure out what is going on by reading the source code of Hypothesis. In hypothesis/strategies/_internal/core.py (version 6.14.1), the only spot I could identify where deferred might be evaluated is in the function as_strategy (Line 956):

def _from_type(thing: Type[Ex]) -> SearchStrategy[Ex]:
    ...

    def as_strategy(strat_or_callable, thing, final=True):

where you check whether a strategy is empty (Line 978):

if strategy.is_empty:
   ...

@Zac-HD
Copy link
Member

Zac-HD commented Jul 8, 2021

Hmm, maybe dropping a deferred in

kwargs[k] = just(p.default) | _from_type(hints[k])
for non-builtin-or-stdlib types would do the trick? So we have kwargs[k] = just(p.default) | deferred(lambda t=hints[k]: _from_type(t)), to capture the loop variable.

(I'll come back for a deeper investigation later)

@mristin
Copy link
Contributor Author

mristin commented Jul 8, 2021

@Zac-HD I don't have the understanding deep enough about that part of the code you posted, but I have a feeling that such a step would be too general? In other words, wouldn't it cause all kinds of unintended behaviors?

I most probably miss something, but isn't there a bug that the deferred is actually de-referenced when you supply it to st.st.register_type_strategy? (Please see my previous comment about the de-referenced deferred strategy above.)

@Zac-HD Zac-HD added bug something is clearly wrong here and removed question not sure it's a bug? questions welcome labels Oct 25, 2021
@Zac-HD
Copy link
Member

Zac-HD commented Nov 29, 2021

With the patch I wrote to defer resolution of type hints in builds(), this works:

class RecursiveClass:
    def __init__(self, value: int, next_node: typing.Optional["SomeClass"]):
        assert value > 0
        self.value = value
        self.next_node = next_node

st.register_type_strategy(
    RecursiveClass,
    st.builds(RecursiveClass, value=st.integers(min_value=1))  # next_node inferred from type annotation
)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something is clearly wrong here
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants