Documentation preview

https://nvidia-merlin.github.io/Transformers4Rec/review/pr-600

I gave it a whirl--including some creative writing on the parameters to the class:

• Two sentences after the code sample in the README: https://mikemckiernan.github.io/Transformers4Rec/review/pr-6/README.html#sample-code-defining-and-training-the-model. New edit, instead of "The preceding code..." I'd like to self edit to "You can modify the preceding code..." so the text is user-focused.
• Put the sample code in an "Example usage" section in the API desc of the class.

I drafted the change as a make-believe PR in my fork. If you like the changes, I can add the commit to this PR. It seemed too presumptuous to add a commit to your PR without checking.

I gave it a whirl--including some creative writing on the parameters to the class:

• Two sentences after the code sample in the README: https://mikemckiernan.github.io/Transformers4Rec/review/pr-6/README.html#sample-code-defining-and-training-the-model. New edit, instead of "The preceding code..." I'd like to self edit to "You can modify the preceding code..." so the text is user-focused.
• Put the sample code in an "Example usage" section in the API desc of the class.

I drafted the change as a make-believe PR in my fork. If you like the changes, I can add the commit to this PR. It seemed too presumptuous to add a commit to your PR without checking.

@mikemckiernan it sounds fine to me but we also change the masking to None in the input_module for BC task. so that'd be also different than the next-item prediction task. I understand your point. you think the code snippets are very similar, and big chunk is repeated twice.. But I'd say better to repeat it than defining it insufficiently. what do you think?

@mikemckiernan it sounds fine to me but we also change the masking to None in the input_module for BC task. so that'd be also different than the next-item prediction task. I understand your point. you think the code snippets are very similar, and big chunk is repeated twice.. But I'd say better to repeat it than defining it insufficiently. what do you think?

I don't know if I'll persuade you to my thinking, but I'll try. With so much similarity, the reader is using brain power to detect what is different (and wondering why two tasks are so overwhelmingly similar) than focusing on learning the concept--learning how to write the code to run a particular type of prediction task, classification, and so on.

I think it'd be better to include more of the sample code in the API reference section, as long as it is typical that most folks will want to specify those blocks. I made the proposed change in the API desc.

My feeling is that we can't provide end-to-end examples for all models. The introductory material in the README should show common, typical, or simple examples. For less common models, code, and so on, the API reference is pretty ideal--can anyone really argue with finding sample code about binary classification anywhere other than the API ref for the binary classification class?

do we not need the schema.pb file in the examples?

@bschifferer thanks for the comment. for the getting-started this schema file is not required, it is generated from NVT and read in then. We kept it there due to unit test, now we changed the unit test and it is run via testbook, we dont need this file either. This file creates confusion for the users so let's remove it :)