TST/CLN: parameterize/dedup replace test2 #41501
Conversation
mzeitlin11
added
Clean
replace
There was a problem hiding this comment.
Thanks @mzeitlin11 generally lgtm. can you rename the variables to avoid confusion and maybe move the result assignment outside a conditional (by either conditional setting arguments or skipping to avoid the redundant permutation )
| ], | ||
| ) | ||
| @pytest.mark.parametrize( | ||
| "regex,value", [(r"\s*\.\s*", np.nan), (r"\s*(\.)\s*", r"\1\1\1")] |
There was a problem hiding this comment.
regex -> to_replace to match DataFrame.replace parameter name
| "regex,value", [(r"\s*\.\s*", np.nan), (r"\s*(\.)\s*", r"\1\1\1")] | ||
| ) | ||
| @pytest.mark.parametrize("compile_regex", [True, False]) | ||
| @pytest.mark.parametrize("use_regex_value_arg", [True, False]) |
There was a problem hiding this comment.
use_regex_value_arg -> regex (or regex_kwarg)
There's also a redundant permutation here can maybe add a skip for that case instead of calling with regex=True explictily.
There was a problem hiding this comment.
Renamed arg. Could you please elaborate on the redundant permutation here? Are you referring to when compile_regex=True and regex_kwarg=False (so regex=True does not really need to be specified?)
| assert return_value is None | ||
| tm.assert_frame_equal(dfobj, res.fillna(".")) | ||
| if use_regex_value_arg: | ||
| result = df.replace(regex=regex, value=value, inplace=inplace) |
There was a problem hiding this comment.
can you put the arguments in the same order as the parameters
|
wow, lgtm. |
|
thanks @mzeitlin11 |
No description provided.