Remove the possibility in DFtoVW to provide constant values #2601
Conversation
| @@ -1458,7 +1441,7 @@ def __init__(self, colname, expected_type, min_value=None): | |||
|
|
|||
| Parameters | |||
| ---------- | |||
| colname: str/int/float | |||
| colname: any hashable type (str/int/float/tuple/etc.) | |||
There was a problem hiding this comment.
not actually link to the current PR. It's just that pandas accept all these types as colname (same as allowed keys for dict type I think).
There was a problem hiding this comment.
do you think it's worthwhile adding that as a comment in the code itself to avoid confusion with vw format, etc
There was a problem hiding this comment.
I think I could specify the type directly as colname: any valid pandas's column name (str/int/float/etc.)
There was a problem hiding this comment.
agree with being more explicit mentioning panda's here
python/vowpalwabbit/pyvw.py
Outdated
|
|
||
| name = AttributeDescriptor("name", expected_type=(int, float)) |
There was a problem hiding this comment.
semirelated to this PR but @lalo what do you think of changing the name arg into label (and same for MulticlassLabel, etc.)
There was a problem hiding this comment.
I think that it stands with our current docs, if you refer to https://github.com/VowpalWabbit/vowpal_wabbit/wiki/Input-format you can see that the terminology uses 'label' even for multiclass.
There was a problem hiding this comment.
Ok i've just change it !
|
Great work and congrats on your fifth pull request to VW @etiennekintzler |
|
Thanks ! 👍 |
This PR fix issue #2600.
Changes
Featureclass, the name provided can only be a constant string now and not a column name. Indeed, since now the user cannot choose whether the argumentnameis a constant or a column name, it's by default a constant.