refactor tree encoder: add encoding dict, param unseen, inverse transform

[solegalli]

closes #729
closes #728
closes #588

[solegalli]

Hey @VascoSch92 @glevv @93lorenzo

If you have a minute to spare, a quick look to see that there is nothing big that I am missing will be much appreciated :)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.19%. Comparing base (46155a0) to head (c0c1407).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #757   +/-   ##
=======================================
  Coverage   98.18%   98.19%           
=======================================
  Files         105      105           
  Lines        4074     4093   +19     
  Branches      795      802    +7     
=======================================
+ Hits         4000     4019   +19     
  Misses         29       29           
  Partials       45       45           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[solegalli]

Thank you guys! @93lorenzo @glevv @VascoSch92

Really appreciate it :)

[glevv]

Missed one

[glevv]

This one should also be fixed, but I'm not sure why there would be an error

[solegalli]

Is this comment new? I don't think I left any record in the file I committed last. Just doubled checked and didnt find any left.

The error comes from the values within parametrize, when I pass a list or a tuple, say ["raise", "ignore"] and then that tuple is passed as {unseen}, it doesn't like it. But it resolved with re.escape(message here)

[glevv]

Also look at this one

[solegalli]

Yep, I didn't miss them. I changed them to match=msg, but then these 2 returned an error which I didn't understand and I was lazy to resolve. Although the messages were identical, the test failed, and it said something about did you want to escape something? Did you come across this before? Probably something to do with regex. which is not my forte :/

[glevv]

both lines of the msg should be f-strings in both cases

[solegalli]

Hmm, not sure I follow: This is the error

E       AssertionError: Regex pattern did not match.
E        Regex: 'During the encoding, NaN values were introduced in the feature(s) var_A, var_B.'
E        Input: 'During the encoding, NaN values were introduced in the feature(s) var_A, var_B.'
E        Did you mean to `re.escape()` the regex?

And this is the code:

def test_fit_errors_if_new_cat_values_and_unseen_is_raise_param(df_enc):
    encoder = DecisionTreeEncoder(unseen="raise", regression=False)
    encoder.fit(df_enc[["var_A", "var_B"]], df_enc["target"])
    X = pd.DataFrame(
        {
            "var_A": ["A", "ZZZ", "YYY"],
            "var_B": ["C", "YYY", "ZZZ"],
        }
    )
    var_ls = "var_A, var_B"
    msg = (
        f"During the encoding, NaN values were introduced in the feature(s) {var_ls}."
    )
    # new categories will raise an error
    with pytest.raises(ValueError, match=msg):
        encoder.transform(X)
    # assert str(record.value) == msg

What am I doing wrong?

[glevv]

Oh, I see. Match is a regex pattern search, so it gets confused by (s). Put an escape character (should be \) like this:

rf"During the encoding, NaN values were introduced in the feature\(s\) {var_ls}."

Other options would be calling re.escape() on msg:

re.escape(f"During the encoding, NaN values were introduced in the feature(s) {var_ls}.")

or shortening the msg (since its not an equality but a regex match it would work):

f"{var_ls}."

[glevv]

Updated the comment

[solegalli]

Life saver @glevv thank you so much! Now it works :)

[solegalli]

@glevv fixed here