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

Add docstrings #1110

Merged
merged 15 commits into from
Jun 12, 2023
Merged

Add docstrings #1110

merged 15 commits into from
Jun 12, 2023

Conversation

edknv
Copy link
Contributor

@edknv edknv commented May 23, 2023

Adding docstrings from the doc bash.

gabrielspmoreira and others added 4 commits May 21, 2023 08:23
* Add docstrings to Encoder

* Add docstrings to ItemRetrievalScorer

* Add docstrings to Model

* Fix docstring for TwoTowerModel

* Add docstring to YoutubeDNNRetrievalModelV2

* Add docstrings to L2Norm

* Add docstrings to ContinuousFeatures

* Add docstrings to AverageEmbeddingsByWeightFeature

* Add docstrings to ReplaceMaskedEmbeddings

* Add docstrings to SequenceEmbeddingFeatures

* Add docstrings to EmbeddingTable

* lint

* lint

* lint
@edknv edknv added documentation Improvements or additions to documentation enhancement New feature or request labels May 23, 2023
@edknv edknv added this to the Merlin 23.05 milestone May 23, 2023
@github-actions
Copy link

Documentation preview

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

@@ -526,23 +627,90 @@ def fit(self, *args, **kwargs):

@tf.keras.utils.register_keras_serializable(package="merlin.models")
class EmbeddingEncoder(Encoder):
"""Creates an Encoder from an EmbeddingTable.
Typically used with RetrievalModelV2.
Parameters
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need a whitespace line before Parameters and before Returns or else it won't format right.

Here's an example of how this looks when the spacing is missing: https://nvidia-merlin.github.io/models/review/pr-1110/generated/merlin.models.tf.EmbeddingEncoder.html#merlin.models.tf.EmbeddingEncoder

Other places where this occurs:

  • ModelBlock
  • MetricsComputeCallback
  • _maybe_convert_merlin_dataset
  • get_task_name
  • _get_col_set_by_tags
  • _set_task_block
  • default_binary_metrics
  • default_categorical_prediction_metrics

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I resolved these and some other instances of this pattern in the repo.

@edknv edknv merged commit cba6697 into main Jun 12, 2023
@edknv edknv deleted the tf/doc_strings_update branch June 12, 2023 11:10
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 enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Task] Add/Fix the doc strings for TF based session based API documentation
5 participants