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

remove transform parameter from loader #935

Closed
wants to merge 2 commits into from

Conversation

edknv
Copy link
Contributor

@edknv edknv commented Dec 21, 2022

A follow-up to #845

Goals ⚽

Remove transform parameter from merlin.models.tf.Loader.

Implementation Details 🚧

The transform parameter was removed when we migrated the loader to use the dataloader package, in favor of dataloader's transforms (with an s). As a result, the transform parameter will not do anything even if it's used, and not removing it was a miss in the previous PR. This PR properly cleans up the unused parameter.

Testing Details πŸ”

Existing tests.

@edknv edknv added the chore Maintenance for the repository label Dec 21, 2022
@edknv edknv self-assigned this Dec 21, 2022
@github-actions
Copy link

Documentation preview

https://nvidia-merlin.github.io/models/review/pr-935

@gabrielspmoreira
Copy link
Member

@edknv I placed a similar PR #937 and only after saw your PR :) . Mine also updates some tests that were using transform and removes also unused multi_target_as_dict.

@edknv
Copy link
Contributor Author

edknv commented Dec 21, 2022

Closing in favor of #937, which also should fix additional issues.

@edknv edknv closed this Dec 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Maintenance for the repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants