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

Convert a test to use Hypothesis #1507

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

etellman
Copy link

@etellman etellman commented Aug 3, 2023

This is a proof of concept for Hypothesis tests. I selected a test that was easy to convert and replaced the existing test with hard-coded values with a few tests that use Hypothesis.

Since it was pretty easy to do, I also added tests for complex inputs. One thing I discovered is that cscalar doesn't work for any of the numpy functions. Hypothesis inevitably finds a value that can't be represented precisely and the test fails. All the functions work with zscalar.

see: #197

Thank you for opening a PR!

Here are a few important guidelines and requirements to check before your PR can be merged:

  • [ x] There is an informative high-level description of the changes.
  • [ x] The description and/or commit message(s) references the relevant GitHub issue(s).
  • [x ] pre-commit is installed and set up.
  • [ x] The commit messages follow these guidelines.
  • [x ] The commits correspond to relevant logical changes, and there are no commits that fix changes introduced by other commits in the same branch/BR.
  • [x ] There are tests covering the changes introduced in the PR.

Don't worry, your PR doesn't need to be in perfect order to submit it. As development progresses and/or reviewers request changes, you can always rewrite the history of your feature/PR branches.

If your PR is an ongoing effort and you would like to involve us in the process, simply make it a draft PR.

This is a proof of concept for Hypothesis tests. I selected a test that
was easy to convert and replaced the existing test with hard-coded
values with a few tests that use Hypothesis.

Since it was pretty easy to do, I also added tests for complex inputs.
One thing I discovered is that cscalar doesn't work for any of the
numpy functions. Hypothesis inevitably finds a value that can't be
represented precisely and the test fails. All the functions work with
zscalar.

see: aesara-devs#197
@brandonwillard
Copy link
Member

This looks great, thanks! Will review shortly.

Remove unused imports and fix one formatting issue.
@etellman
Copy link
Author

etellman commented Aug 3, 2023

This looks great, thanks! Will review shortly.

Sounds good. Let me know what you think.

It looks like I missed a few Black issues, so I fixed them (I hope) with this commit: fa832d4

@@ -56,6 +56,7 @@ dependencies = [
"cons",
"typing_extensions",
"setuptools >=48.0.0",
"hypothesis",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might need to add this to the test set up script (i.e. here).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added it. I'm not sure how to test it, as I'm a little fuzzy on what this script is used for, but I guess we'll find out when something happens on GitHub. :^)

Add Hypothesis installation to test.yaml in the GitHub workflow.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants