Small fixes in getting-started ETL and training notebooks and fix tuple error in serving notebook #586
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
rnyak
changed the title
Documentation previewhttps://nvidia-merlin.github.io/Transformers4Rec/review/pr-586 |
| @@ -28,7 +28,8 @@ | |||
| SESSION_PATH = "examples/getting-started-session-based/" | |||
|
|
|||
|
|
|||
| @pytest.mark.skipif(find_spec("cudf") is None, reason="needs cudf") | |||
| # @pytest.mark.skipif(find_spec("cudf") is None, reason="needs cudf") | |||
| @pytest.mark.skip(reason="there is a testbook version of this test") | |||
| def test_session(tmpdir): | |||
There was a problem hiding this comment.
To double-check, will this test always be skipped?
If yes, can we remove it as it is replaced by the testbook version?
There was a problem hiding this comment.
@jperez999 since I am replacing this test with the testbook version I added mark.skip into the existing test, but we can remove it as well. what do u think? thanks.
|
The PR looks good to me. Do we have no Jenkins GitHub Action for it? |
Thanks for reviewing. If I understood your question correctly : AFAIK, we do no longer use Jenkins for PR testing |
|
it ran the new test and skipped the old one https://github.com/NVIDIA-Merlin/Transformers4Rec/actions/runs/3795493384/jobs/6454668622#step:3:326 |
This PR:
03notebook.01and02notebooks using testbook.Note: if we want a unit test for serving notebook, we would need systems here. currently we dont couple TF4Rec to systems, that's why I removed that part in the unit test code. The decision is to do that in Merlin repo, instead of TF4Rec repo.