docs: Remove inheritable docstrings

[dangotbanned]

What type of PR is this? (check all applicable)

• 💾 Refactor
• ✨ Feature
• 🐛 Bug Fix
• 🔧 Optimization
• 📝 Documentation
• ✅ Test
• 🐳 Other

Related issues

• Commitment to keep package size down #1942
• docs: make docstrings concise in expr_str, expr_dt, series_str, ... and more #2068
• docs: Make DataFrame and LazyFrame docstrings shorter and more concise #1939

Checklist

• Code follows style guide (ruff)
• Tests added
• Documented the changes

If you have comments or can explain your changes, please do so below

https://docs.python.org/3/library/inspect.html#inspect.getdoc

AFAIK, adding docstrings in the v1 classes/methods is probably doing more harm than good:

• Best-case, we're duplicating information that we need to keep in sync
• Worst-case, we override a docstring that had examples with a version without examples

I think we should only be adding docstrings if they are different to what is on main.
Things like deprecations, breaking changes, etc

Examples

Before

narwhals/narwhals/stable/v1/__init__.py

Lines 233 to 244 in f47ad1f

	def to_dict(
	self, *, as_series: bool = True
	) -> dict[str, Series[Any]] | dict[str, list[Any]]:
	"""Convert DataFrame to a dictionary mapping column name to values.
	Arguments:
	as_series: If set to true ``True``, then the values are Narwhals Series,
	otherwise the values are Any.
	Returns:
	A mapping from column name to values / Series.
	"""

After

narwhals/narwhals/stable/v1/__init__.py

Lines 183 to 185 in a571160

	def to_dict(
	self, *, as_series: bool = True
	) -> dict[str, Series[Any]] | dict[str, list[Any]]:

Tasks

• Figure out why only 3.13 is working, but all earlier versions break formatting
  • Resolved in (04eaf9b)
• Rewrite stable_api_test with less copy/pasting
• Remove repeat docs from v1
  • DataFrame
  • LazyFrame
  • Series
  • Expr
  • Schema
  • Datetime
  • Duration
• Handle the class-level docs
  • See docs: Remove inheritable docstrings #2427 (comment)

[dangotbanned]

Ahhh we had a nice streak of no unrelated downstream CI issues going there for a bit 😢

https://github.com/narwhals-dev/narwhals/actions/runs/14629063988/job/41047719471?pr=2427

Update

Resolved in (#2428)

[dangotbanned]

Feedback please!

We have a few options for dealing with the missing class-level docstrings.

Important

Method-level docstrings are fine

1. Revert that part

Simplest option, lowers my code-golf score but no issues beyond that 🙂

2. Getting funky

I can get them to appear statically, but only when the constructor is called:

Locally, I've adapted the typing I used in altair.
The usage is simply decorating <subclass>.__init__ with the parent's context

Bad news

But that doesn't extend to hovering over the class itself 😞

3. Getting funkier

If we did something more dynamic we could get the class-level doc.
However, that would only be displayed in a notebook/kernel/live environment.

It might be possible to combine 2 & 3, but I don't know if they'd interact well across all environments.

Personal note

99% of the time I have kernel completions disabled - since I find them too noisy with pylance at the same time.

So while I'm presenting this as an option - I'd prefer an alternative for my own sake 😄

"jupyter.enableKernelCompletions": false

[MarcoGorelli]

thanks @dangotbanned ! tbh I don't think we need class-level docstrings too much, no objections to removing them

[dangotbanned]

thanks @dangotbanned ! tbh I don't think we need class-level docstrings too much, no objections to removing them

Thanks for taking a look @MarcoGorelli

I can't tell if you were saying either:

• You're in favor of a particular option in (docs: Remove inheritable docstrings #2427 (comment))
• or a fourth path where we remove the class-level docstrings 😄

For the example I gave in (#2427 (comment)) I can understand why they might not seem like a big loss - since we're just missing an admonition.

However, there's also Datetime and Duration which have a bit more utility

Main

V1 (currently in this PR)

V1 (using 2. Getting funky)

Also includes an example we didn't have before

Possible with minimal changes to Datetime

[MarcoGorelli]

thanks @dangotbanned , changes to docstrings look good

could you summarise the gist of the change in tests/stable_api_test.py please?

[dangotbanned]

could you summarise the gist of the change in tests/stable_api_test.py please?

Getting gist-y with it

@MarcoGorelli the changes are centred on this new helper generator function, which mostly is just replacing the repetition in each of:

• test_dataframe_docstrings
• test_lazyframe_docstrings
• test_series_docstrings

Previous

narwhals/tests/stable_api_test.py

Lines 107 to 157 in 8944823

	def test_dataframe_docstrings() -> None:
	pytest.importorskip("polars")
	import polars as pl
	stable_df = nw_v1.from_native(pl.DataFrame())
	df = nw.from_native(pl.DataFrame())
	api = [i for i in df.__dir__() if not i.startswith("_")]
	for item in api:
	assert remove_docstring_examples(
	getattr(stable_df, item).__doc__.replace(
	"import narwhals.stable.v1 as nw", "import narwhals as nw"
	)
	) == remove_docstring_examples(getattr(df, item).__doc__), item
	def test_lazyframe_docstrings() -> None:
	pytest.importorskip("polars")
	import polars as pl
	stable_df = nw_v1.from_native(pl.LazyFrame())
	df = nw.from_native(pl.LazyFrame())
	api = [i for i in df.__dir__() if not i.startswith("_")]
	for item in api:
	if item in {"schema", "columns"}:
	# to avoid performance warning
	continue
	if item in {"tail", "gather_every"}:
	# deprecated
	continue
	assert remove_docstring_examples(
	getattr(stable_df, item).__doc__.replace(
	"import narwhals.stable.v1 as nw", "import narwhals as nw"
	)
	) == remove_docstring_examples(getattr(df, item).__doc__)
	def test_series_docstrings() -> None:
	pytest.importorskip("polars")
	import polars as pl
	stable_df = nw_v1.from_native(pl.Series(), series_only=True)
	df = nw.from_native(pl.Series(), series_only=True)
	api = [i for i in df.__dir__() if not i.startswith("_")]
	for item in api:
	if getattr(df, item).__doc__ is None:
	continue
	assert remove_docstring_examples(
	getattr(stable_df, item).__doc__.replace(
	"import narwhals.stable.v1 as nw", "import narwhals as nw"
	)
	) == remove_docstring_examples(getattr(df, item).__doc__), item

Notes

• Used dir(...) instead of __dir__()
• Collapsed the various if <predicate>: continue statements into one that can be parametrized
  • The groups in test_lazyframe_docstrings with comments became the name of variables passed to *exclude
    • This is intended to make the code self-documenting, but it seems to have not worked 😢
• Used inspect.getdoc(...) instead of __doc__
  • The latter broke following (0a5c5e7), whereas the next commit (64f6ef9) worked as documented

[dangotbanned]

https://github.com/narwhals-dev/narwhals/actions/runs/14693968316/job/41233109700?pr=2427

@MarcoGorelli this seems to have popped up after merging (#2325) 🤔

[XPASS(strict)] 
 =========================== short test summary info ============================
FAILED tests/group_by_test.py::test_group_by_expr[polars[lazy]-0]
= 1 failed, 9881 passed, 147 skipped, 519 xfailed, 1 xpassed in 222.44s (0:03:42) =
Error: Process completed with exit code 1.

[MarcoGorelli]

there's just been a new polars release, could well be that

thanks @dangotbanned !