input checks and unit tests for TwoColumnOperatorTransformer #183
Conversation
| ValueError, | ||
| match="columns must be a list containing two column names", | ||
| ): | ||
| TwoColumnOperatorTransformer("mul", [], "c", pd_method_kwargs={"axis": 1}) |
There was a problem hiding this comment.
Please can you check this with a list of length one?
An empty list will evaluate to none and so could accidentally be picking up some other behaviour (I'm maybe being over cautious here)
There was a problem hiding this comment.
all good with both empty and length one it seems
There was a problem hiding this comment.
Can we parameterise this test with both empty list and ['a']? https://docs.pytest.org/en/7.1.x/how-to/parametrize.html
There was a problem hiding this comment.
included both tests for empty and length 1 a25f439
CHANGELOG.rst
Outdated
| @@ -34,6 +34,12 @@ Removed | |||
| this functionality. This required updating a lot of test files. | |||
| - The `columns_set_or_check()` method from BaseTransformer. With the above change it was no longer necessary. Subsequent updates to nominal transformers and their tests were required. | |||
|
|
|||
| 1.2.3 (2024-02-23) | |||
There was a problem hiding this comment.
Changelog is out of date, please can you pull the latest changes from main to your branch :)
There was a problem hiding this comment.
main is on 1.3.0 now:
https://github.com/lvgig/tubular/blob/main/CHANGELOG.rst
Need to pull changes
There was a problem hiding this comment.
Can you move this entry to under 1.3.0 please @ChaitanMohr ?
There was a problem hiding this comment.
d1d8044 changed version here
There was a problem hiding this comment.
Hm looks like the changelog is unchanged?
There was a problem hiding this comment.
updated changelog 2544a1f
|
Hi @ChaitanMohr, please add a PR description. I know we have the linked issue but a descriptive PR description makes it clear what occured in this PR for future contributors. |
This PR aims to edit the existing TwoColumnOperatorTransformer to ensure that: