Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] LambdaOp must be explicit #163

Closed
mattf opened this issue Mar 25, 2022 · 2 comments · Fixed by #166
Closed

[BUG] LambdaOp must be explicit #163

mattf opened this issue Mar 25, 2022 · 2 comments · Fixed by #166
Assignees
Labels
documentation Improvements or additions to documentation
Milestone

Comments

@mattf
Copy link

mattf commented Mar 25, 2022

https://github.com/NVIDIA-Merlin/Merlin/blob/main/examples/getting-started-movielens/02-ETL-with-NVTabular.ipynb

ratings = nvt.ColumnGroup(["rating"]) >> (lambda col: (col > 3).astype("int8"))

this code will result in an exception when trying to call create_node on a function

a fix is to explicitly use LambdaOp-

ratings = nvt.ColumnGroup(["rating"]) >> nvt.ops.LambdaOp(lambda col: (col > 3).astype("int8"))

a better fix would be to type detect the function and wrap it with a LambdaOp

@karlhigley
Copy link
Contributor

I believe the functionality to allow unwrapped lambdas was intentionally deprecated and removed in order to support multiple types of graphs and nodes. It looks like the examples in the Merlin repo haven't been updated in a while though; the version of the same notebook in the NVTabular repo does use an explicit LamdaOp. Seems like the path forward here to might be to migrate the examples from the NVTabular repo to the Merlin repo. What do you think, @rnyak?

@karlhigley karlhigley added the documentation Improvements or additions to documentation label Mar 25, 2022
@karlhigley karlhigley added this to the Merlin 22.04 milestone Mar 25, 2022
@mattf
Copy link
Author

mattf commented Mar 26, 2022

it looks like https://github.com/NVIDIA-Merlin/Merlin/tree/main/examples/getting-started-movielens is stale compared to https://github.com/NVIDIA-Merlin/NVTabular/tree/main/examples/getting-started-movielens

if you ask me, i'll say to move the NVTabular version over to the Merlin repository. it's more discoverable and natural.

@karlhigley karlhigley linked a pull request Mar 26, 2022 that will close this issue
@karlhigley karlhigley self-assigned this Mar 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants