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

Support consumes(some_bundle).filter(...) for stateful tests #3944

Open
Zac-HD opened this issue Mar 31, 2024 · 1 comment · May be fixed by #4084
Open

Support consumes(some_bundle).filter(...) for stateful tests #3944

Zac-HD opened this issue Mar 31, 2024 · 1 comment · May be fixed by #4084
Labels
enhancement it's not broken, but we want it to be better

Comments

@Zac-HD
Copy link
Member

Zac-HD commented Mar 31, 2024

Inspired by pydata/xarray#8658 (comment): you can't currently filter a consumes(bundle) strategy, which would be useful:

  • consumes(bundle).filter(fn) basically works, but the retries in FilteredStrategy.do_draw() mean that you're likely to consume more elements from the bundle than intended. That's semantically OK - maybe we just chose never to refer to them again later - but can be a performance hit if acceptable elements are rare, and you're already missing the filtering-sampled_from fast path.
  • consumes(bundle.filter(fn)) is disallowed by the types involved, but not for any especially principled reason.

So... let's just fix that! A BundleReferenceStrategy is practically a SampledFromStrategy already, so I propose that we make Bundle a subclass of SampledFromStrategy. That would give us automatic support for both cases above, as well as all the nice logic for efficient handling of filter (and map) transforms, unique collections, and so on.

(we might as well inline BundleReferenceStrategy into Bundle while we're at it; there's no real reason to have it separate)

@Zac-HD Zac-HD added the enhancement it's not broken, but we want it to be better label Mar 31, 2024
@pspacek
Copy link

pspacek commented Apr 25, 2024

This would be an useful extension - I just hit that limitation while working on an unrelated test.

@Zac-HD Zac-HD mentioned this issue May 18, 2024
22 tasks
@reaganjlee reaganjlee linked a pull request Aug 12, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement it's not broken, but we want it to be better
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants