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

[AG-1105] Data-Driven Test for transform_genes_biodomains #74

Merged
merged 10 commits into from
Jun 1, 2023

Conversation

BWMac
Copy link
Contributor

@BWMac BWMac commented May 23, 2023

This PR implements the TestTransformGenesBiodomains class in transform_genes_biodomains.py. The new class is made up of some attribute data objects used for the parameterized testing, and two test functions, test_transform_genes_biodomains_should_pass and test_transform_genes_biodomains_should_fail. Each test imports the example input files, attempts to run genes_biodomains.transform_genes_biodomains, and checks the result. In the passing case, the resulting output_df is checked against the expected_output which is provided by the example output files. In the failing case, we expect the function call to result in a KeyError which is verified.

The parameterized testing runs are also annotated with IDs that describe which condition is being tested. When you run

pytest tests/transform/test_genes_biodomains.py -v

the output from the new tests looks like

tests/transform/test_genes_biodomains.py::TestTransformBiodomains::test_transform_genes_biodomains_should_pass[Pass with good data] PASSED [ 60%]
tests/transform/test_genes_biodomains.py::TestTransformBiodomains::test_transform_genes_biodomains_should_pass[Pass with imperfect data] PASSED [ 80%]
tests/transform/test_genes_biodomains.py::TestTransformBiodomains::test_transform_genes_biodomains_should_fail[Fail with bad data] PASSED [100%]

Copy link
Member

@thomasyu888 thomasyu888 left a comment

Choose a reason for hiding this comment

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

Awesome work - I know this is WIP, but here are some initial thoughts.

tests/__init__.py Outdated Show resolved Hide resolved
tests/transform/test_genes_biodomains.py Outdated Show resolved Hide resolved
tests/transform/test_genes_biodomains.py Outdated Show resolved Hide resolved
tests/transform/test_genes_biodomains.py Outdated Show resolved Hide resolved
@BWMac BWMac changed the title [AG-1105] Test for transform_genes_biodomains [AG-1105] Data-Driven Test for transform_genes_biodomains May 24, 2023
@BWMac BWMac marked this pull request as ready for review May 24, 2023 16:35
@BWMac BWMac marked this pull request as draft May 24, 2023 16:35
@BWMac BWMac requested a review from thomasyu888 May 24, 2023 16:35
@BWMac BWMac marked this pull request as ready for review May 24, 2023 16:39
Copy link
Member

Choose a reason for hiding this comment

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

Point of discussion: Should we have these test files within transform folders:

tests/test_data/biodomains/input
tests/test_data/biodomains/output

Copy link
Member

@thomasyu888 thomasyu888 left a comment

Choose a reason for hiding this comment

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

🪟 I like this framework, let's see what Jess/Jaclyn think!

@BWMac BWMac requested review from JessterB and jaclynbeck-sage May 24, 2023 18:28
Copy link
Contributor

@JessterB JessterB left a comment

Choose a reason for hiding this comment

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

LGTM, but we should wait for Jaclyn to take a look too.

Out of curiosity what do the % values after the test output mean? Does that represent the % of the tests that have passed so far? If so, why is the first test 60% of the tests?

...PASSED [ 60%]
...PASSED [ 80%]
....PASSED [100%]

@BWMac
Copy link
Contributor Author

BWMac commented May 24, 2023

Thanks @JessterB ! I believe that is what the % values mean. This looks weird here because I copied only the output lines that pertain to these new tests. The full output looks like this:

tests/transform/test_genes_biodomains.py::TestCountGroupedTotal::test_count_grouped_total_one_group PASSED                      [ 20%]
tests/transform/test_genes_biodomains.py::TestCountGroupedTotal::test_count_grouped_total_two_groups PASSED                     [ 40%]
tests/transform/test_genes_biodomains.py::TestTransformBiodomains::test_transform_biodomains_should_pass[Pass with good data] PASSED [ 60%]
tests/transform/test_genes_biodomains.py::TestTransformBiodomains::test_transform_biodomains_should_pass[Pass with imperfect data] PASSED [ 80%]
tests/transform/test_genes_biodomains.py::TestTransformBiodomains::test_transform_biodomains_should_fail[Fail with bad data] PASSED [100%]

With the tests for genes_biodomains.count_grouped_total also included.

Copy link
Member

@thomasyu888 thomasyu888 left a comment

Choose a reason for hiding this comment

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

🔥 LGTM! I'm going to pre-approve first. Can you add to the CONTRIBUTING.md some instructions on how to contribute these transform tests? (There's a PR open for the contributing guide)

It doesn't have to be perfect, but it'll serve as a starting place for Jess/Jaclyn.

@BWMac
Copy link
Contributor Author

BWMac commented Jun 1, 2023

@thomasyu888 I added instructions for contributing transform tests to #72. Merging this one now.

@BWMac BWMac merged commit 2ef1c13 into dev Jun 1, 2023
@BWMac BWMac deleted the bwmac/AG-1105/genes_biodomains_test branch June 1, 2023 12:42
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.

4 participants