max is by default over the keys. This also avoids a type error because dict.get actually returns an Optional type.

[DBrughmans]

max is by default over the keys. This also avoids a type error because dict.get actually returns an Optional type.

Originally posted by @jklaise in #541 (comment)

This change is causing me issues in newer versions. This change does not result in the same output. I think the goal is to find the key which is paired with the highest value. The new method just returns the key which is the highest value.

cat_vars
{0: 2, 2: 3, 5: 8, 13: 2, 15: 2, 17: 2, 19: 3}
max(cat_vars)
19
max(cat_vars,key=cat_vars.get)
5

Versions after this commit do not work with categorical features for me.

[jklaise]

@DBrughmans you're absolutely correct, this is a bug and an oversight from me. The original function of key=cat_vars.get was to find the categorical variable with the most number of categories so that we can allocate a tensor big enough to hold information of category embeddings. With the buggy change this is no longer the case. By some accident, in the examples we have both versions of the max give the same answer since the variable with the most categories happens also to have the highest key. Compounded with the fact that the variable is named max_key explains why I thought the behaviour should be the same when it clearly isnt.

I will revert the bug in an upcoming PR and look to release version 0.6.5 asap. Apologies for causing trouble.

[jklaise]

@DBrughmans I've published version 0.6.5 on PyPI which reverts the bug, if you upgrade, things should work as before. Thanks again for bringing this to our attention.

[DBrughmans]

Thanks for the fast response!