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 random sampling for unit testing #23724

Closed
roed314 opened this issue Aug 26, 2017 · 18 comments
Closed

Allow random sampling for unit testing #23724

roed314 opened this issue Aug 26, 2017 · 18 comments

Comments

@roed314
Copy link
Contributor

roed314 commented Aug 26, 2017

#16244 removed random sampling, but there are times when it would be nice to have. This ticket adds it back in, but optionally.

CC: @nthiery

Component: doctest framework

Author: David Roe

Branch/Commit: 5d4f93b

Reviewer: Julian Rüth

Issue created by migration from https://trac.sagemath.org/ticket/23724

@roed314 roed314 added this to the sage-8.1 milestone Aug 26, 2017
@roed314
Copy link
Contributor Author

roed314 commented Aug 26, 2017

Branch: u/roed/sampling_unittest

@roed314
Copy link
Contributor Author

roed314 commented Aug 26, 2017

Commit: fb20bca

@roed314
Copy link
Contributor Author

roed314 commented Aug 26, 2017

New commits:

fb20bcaAdd the ability to randomly sample and easier access to cartesian products to sage_unittest

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 28, 2017

Changed commit from fb20bca to 5fc43c7

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 28, 2017

Branch pushed to git repo; I updated commit sha1. New commits:

5fc43c7Fix unmatche paren

@tscrim
Copy link
Collaborator

tscrim commented Aug 29, 2017

comment:4

What about also changing some_tuples for consistency?

@roed314
Copy link
Contributor Author

roed314 commented Aug 29, 2017

comment:5

Changing how? Adding an optional keyword argument to use sampling instead? Something like

def some_tuples(elements, repeat, bound, sampling=False):
    from itertools import islice, product
    from random import sample
    P = product(elements, repeat=repeat)
    if sampling:
        try:
            return random.sample(P, bound)
        except ValueError:
            return P
    else:
        return islice(product(elements, repeat=repeat), bound)

@tscrim
Copy link
Collaborator

tscrim commented Aug 29, 2017

comment:6

I was thinking it would have the same idiom as the TestSuite with a max_samples=None argument, but basically, yes.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 5, 2017

Branch pushed to git repo; I updated commit sha1. New commits:

8542826Unify implementatiion of some_tuples and tester.some_elements; improve implementation for large repeat

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 5, 2017

Changed commit from 5fc43c7 to 8542826

@saraedum
Copy link
Member

saraedum commented Sep 6, 2017

Changed branch from u/roed/sampling_unittest to u/saraedum/sampling_unittest

@saraedum
Copy link
Member

saraedum commented Sep 6, 2017

Reviewer: Julian Rüth

@saraedum
Copy link
Member

saraedum commented Sep 6, 2017

Changed commit from 8542826 to 5d4f93b

@saraedum
Copy link
Member

saraedum commented Sep 6, 2017

New commits:

5d4f93bfix typo

@saraedum
Copy link
Member

saraedum commented Sep 6, 2017

comment:10

I am confused about the patchbots errors. I first thought that was noise but it seems to be consistent for several patchbots.

@saraedum
Copy link
Member

saraedum commented Sep 6, 2017

Work Issues: patchbot errors→positive review

@saraedum
Copy link
Member

saraedum commented Sep 6, 2017

Changed work issues from patchbot errors→positive review to none

@vbraun
Copy link
Member

vbraun commented Sep 11, 2017

Changed branch from u/saraedum/sampling_unittest to 5d4f93b

@vbraun vbraun closed this as completed in b99e683 Sep 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants