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

Consistent args and base transformers #168

Merged
merged 7 commits into from
Feb 21, 2024

Conversation

TommyMatthews
Copy link
Contributor

This PR:

  • sets up the generic testing for columns str and list transformers and adds BaseTransformer and DataFrameMethodTransformer (all transformers from base.py)
  • cleans the test files for these transformers (including writing some new behaviour tests to replace implementation tests)
  • Removes columns = None as an option from BaseTransformer (it was default), off the back of this:
    a) removed columns_set_or_check method from package as this is now redundant
    b) fixed ~60 failing unit tests that instantiated transformers without a columns argument (see note below)
    c) updated the example notebook to reflect the removal of this functionality
  • Changed new_column_name to new_column_names in DataframeMethodTransformer to match standardisation going forward
  • Fixed the tests that the above broke (see note below)

Note on existing implementation tests outside base test suite:
When going to fix broken tests I found that a lot of them were implementation tests. In order to maintain a sensible scope for this feature, my policy was:
If the test is in a test file for one of the transformers I am moving to generic testing: fix it if it isn't covered by generic testing.

Elif the test is/will be covered by the base behaviour tests: delete it - it will be covered at some point once generic testing gets to this module, and until then it's not really adding any value as an implicit test.

Elif the behaviour this implementation test is trying to implicitly test for will not be covered by the base behaviour tests: fix it quickly (with an xfail in mind but didn't need to) and mark testing this behaviour as TODO. Include bringing the test suite up to best practice standards as part of the scope of moving a transformer over to generic testing. Will include in process doc

@TommyMatthews TommyMatthews merged commit d85a7cb into main Feb 21, 2024
1 check passed
@davidhopkinson26 davidhopkinson26 deleted the feature/generic_testing_base branch March 21, 2024 09:22
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.

1 participant