Skip to content

docs: add missing docstrings to Dense methods and to_scipy_coo#3843

Open
RavSinghChandan wants to merge 1 commit into
huggingface:mainfrom
RavSinghChandan:docs/add-missing-docstrings-dense-tensor
Open

docs: add missing docstrings to Dense methods and to_scipy_coo#3843
RavSinghChandan wants to merge 1 commit into
huggingface:mainfrom
RavSinghChandan:docs/add-missing-docstrings-dense-tensor

Conversation

@RavSinghChandan

@RavSinghChandan RavSinghChandan commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds missing docstrings to five public methods/functions across two files. Each docstring follows the existing HuggingFace-style documentation patterns used elsewhere in the codebase.

sentence_transformers/base/modules/dense.py — 4 methods:

Method Added
Dense.forward Args / Returns
Dense.get_embedding_dimension Returns
Dense.get_config_dict Returns
Dense.save Args

The Dense class had a thorough class-level docstring but its individual methods were undocumented, making it harder to understand what each method expects and returns at a glance.

sentence_transformers/util/tensor.py — 1 function:

Function Added
to_scipy_coo Args / Returns / Example

to_scipy_coo sits next to compute_count_vector (which already has a full docstring) but had no documentation of its own.

Tests

No logic changed — documentation only.

ruff check and ruff format --check both pass. typos reports no issues.

Add Args/Returns/Example docstrings to Dense.forward,
Dense.get_embedding_dimension, Dense.get_config_dict, Dense.save
and to_scipy_coo
@RavSinghChandan
RavSinghChandan force-pushed the docs/add-missing-docstrings-dense-tensor branch from 3de3466 to 08d0da0 Compare July 1, 2026 02:59
@tomaarsen

Copy link
Copy Markdown
Member

Hello!

Thanks for the PR. For the Dense methods: forward, get_config_dict, and save already inherit their docstrings from the base Module class, so they aren't really missing. They're also not rendered in the API reference, only the class docstring shows. Simpler modules like Pooling and Normalize intentionally leave these to the inherited base docstrings, and I'd like to keep Dense consistent with that. So I'd like to pass on the Dense docstrings.

On the other hand, to_scipy_coo is a nice addition that I'd be happy to add. Could you trim the PR down to just the to_scipy_coo docstring?

  • Tom

@tomaarsen tomaarsen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

See above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants