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

[Pydeequ 1.0.1] pydeequ.checks.isContainedIn does not accept lambda assertion #88

Open
BrunoFavie opened this issue Dec 10, 2021 · 3 comments

Comments

@BrunoFavie
Copy link

Describe the bug
When running Pydeequ 1.0.1 the test generated by ConstraintSuggestionRunner include tests using the isContainedIn() function that fail during execution.

The cause is that the suggested tests include a lambda assertion which the python function does not accept as it takes 3 positional arguments but the suggested tests has 5

To Reproduce
Steps to reproduce the behavior:

  1. Generate a test statement using a dataset that is incomplete, resulting in a suggestion for a test using isContainedIn() which uses a lambda:
  • Example: 'value'is empty for more then 97% of the records:
  • isContainedIn("value", [""], lambda x: x >= 0.97, "It should be above 0.97!")
  1. Execute the test
  2. Check output for error:
  • TypeError: isContainedIn() takes 3 positional arguments but 5 were given

This issue has been reported before: #65

The cause is that the current implementation of the isContainedIn was edited in 30375bb#diff-783716851e9837b9753e643de1f15e031f79bed4ef27e07ce67eeddc5a3fb2ee but the ConstraintSuggestionRunner was not updated to match the latest implementation.

It is unclear to me whether the suggested test is valid and the isContainedIn function needs to be extended or whether the change was made for a reason and thus the ConstraintSuggestionRunner should be adjusted to leave out the broken tests.

A previously made pull request does show how to revert the change: #58

@jagdeepkalsi
Copy link

+1

This one is pretty important.

@SarbajitNandy
Copy link

I looked at the Check class.
`isContainedIn() function takes only two args. 1st - value, 2nd - list of values.
It doesn't support any assertions.

@didopimentel
Copy link

+1 to this. The suggestion seems to have a bug where code_for_constraint is not valid.

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

No branches or pull requests

4 participants