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

Allow optional fields in fixed_dictionaries #1913

Closed
bmccutchon opened this issue Apr 2, 2019 · 4 comments · Fixed by #2127
Closed

Allow optional fields in fixed_dictionaries #1913

bmccutchon opened this issue Apr 2, 2019 · 4 comments · Fixed by #2127
Labels
new-feature entirely novel capabilities or strategies

Comments

@bmccutchon
Copy link

Currently, I can generate a dictionary with an optional field like this:

st.one_of(
    st.fixed_dictionaries({'spam': st.text()}),
    st.fixed_dictionaries({'spam': st.text(), 'optional_key': st.text()}))

However, this gets tedious with many fields. It would be nice to be able to write:

st.fixed_dictionaries(
   {'spam': st.text()},
   optional={'optional_key': st.text()})

In other words, it would be nice to have a parameter to fixed_dictionaries called "optional" that takes a dictionary of optional fields. Here's my attempt at implementing this:

DrawFn = Callable[[strategies.SearchStrategy], Any]

K = TypeVar('K')

@st.composite
def semi_fixed_dicts(
    draw: DrawFn,
    required_fields: Mapping[K, strategies.SearchStrategy],
    optional_fields: Mapping[K, strategies.SearchStrategy],
) -> Dict[K, Any]:
  main = draw(st.fixed_dictionaries(required_fields))
  optional_items = st.sampled_from(
      [(k, draw(s)) for k, s in six.iteritems(optional_fields)])
  extra = draw(st.lists(optional_items, unique_by=operator.itemgetter(0)))
  main.update(extra)
  return main
@Zac-HD Zac-HD added the new-feature entirely novel capabilities or strategies label Apr 3, 2019
@Zac-HD
Copy link
Member

Zac-HD commented Apr 3, 2019

Sounds good to me!

Some thoughts about how we might validate the arguments: this strategy doesn't take size parameters, and I think we should not add them when we add an optional argument. Error if mappings are of different types. Error if there are any common keys between the dicts. We could consider faking keyword-only arguments with an _ argument and error if it is not None.


Here's a tweaked implementation to tide you over:

T = TypeVar('T')
DrawFn = Callable[[strategies.SearchStrategy[T]], T]
K = TypeVar('K')
V = TypeVar('V')
M = TypeVar('M', bound=Mapping)

@st.composite
def semi_fixed_dicts(
    draw: DrawFn,
    required_fields: M[K, strategies.SearchStrategy[V]],
    optional_fields: M[K, strategies.SearchStrategy[V]] = None,
) -> M[K, V]:
    main = draw(st.fixed_dictionaries(required_fields))
    if optional_fields is None:
        return main
    # Note that the items we sample *must* be in stable order - I'd sort them
    # if possible to ensure we can reproduce order between runs.
    opt_strat = st.sampled_from(list(six.iteritems(optional_fields))).flatmap(
        lambda kv: st.tuples(st.just(kv[0]), kv[1])
    )
    for k, v in draw(st.lists(opt_strat, unique_by=lambda kv: hash(kv[0]))):
        main[k] == v
    return main

It's not terribly elegant, but it will generate and shrink values efficiently. I also haven't checked that Mypy copes with this blizzard of generics, but that's the types that I would want to express for this strategy.

@bmccutchon
Copy link
Author

Instead of sampled_from, would something like this also shrink well?

for k, s in six.iteritems(optional_fields):
  if draw(st.booleans()):
    main[k] = draw(s)

It seems simpler. The generics look good. There are ways to use a decorator to get keyword-only arguments in Python 2 (e.g. poscheck_except here: https://github.com/jeffkaufman/poscheck).

@Zac-HD
Copy link
Member

Zac-HD commented Apr 5, 2019

Not quite so well sadly, for complicated internal reasons. Likely to be well enough if you prefer the simpler code though!

And yeah, I was thinking about that kind of decorator but on further investigation I don't think the obfuscation of signatures (to runtime introspection or in the docs) is worth it - we'll just have to do a hard changeover in version 5.0.

@Zac-HD
Copy link
Member

Zac-HD commented Oct 5, 2019

Some experiments: master...Zac-HD:optional-keys

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-feature entirely novel capabilities or strategies
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants