Skip to content

[ENH] Implement dynamic docstring formula injection for distributions#1024

Open
KaranSinghDev wants to merge 2 commits intosktime:mainfrom
KaranSinghDev:enh/docstring-injection
Open

[ENH] Implement dynamic docstring formula injection for distributions#1024
KaranSinghDev wants to merge 2 commits intosktime:mainfrom
KaranSinghDev:enh/docstring-injection

Conversation

@KaranSinghDev
Copy link
Copy Markdown
Contributor

@KaranSinghDev KaranSinghDev commented Mar 31, 2026

Reference Issues/PRs

#689 #698

What does this implement/fix? Explain your changes.

A robust mechanism to inject distribution-specific LaTeX formulas into the generic public methods of BaseDistribution (e.g., pdf, cdf, mean, var, energy).

Key things done:

  1. Indentation-Aware Engine: Automatically matches base docstring indentation to ensure valid Sphinx rendering.
  2. Inheritance Safety: Uses a factory wrapper with functools.wraps in init_subclass to preserve metadata and handle multi-level inheritance safely.
  3. Demos: Fully implemented hooks for Normal, Rayleigh, Exponential, and Laplace.

Fallback (Weibull PDF)
image

Injected Math (Rayleigh PDF - custom)
image

Does your contribution introduce a new dependency? If yes, which one?

No

What should a reviewer concentrate their feedback on?

1.The robustness of the init_subclass implementation.
2. The indentation logic in _inject_formula_doc which was designed to fix the rendering failures discussed in #698.
3. The demonstration implementations in Normal, Rayleigh, Exponential, and Laplace.

Did you add any tests for the change?

Implemented a comprehensive suite in skpro/distributions/tests/test_docstring_injection.py verifying:
1.Injection in hooked classes.
2.Clean fallback (placeholder removal) in unhooked classes across all 12 methods.
3. Execution safety and metadata integrit

Any other comments?

I have implemented the hooks for 4 major distributions. If this approach is decent, I would be happy to extend the formulas to the rest of the distribution registry.
@fkiraly, please let me know if this mechanism meets the project's standards for the documentation pipeline.

P.S @fkiraly this could have been done earlier but I was submitting the abstract of my research project thus the delay.

PR checklist

For all contributions
  • I've added myself to the list of contributors with any new badges I've earned :-)
    How to: add yourself to the all-contributors file in the skpro root directory (not the CONTRIBUTORS.md). Common badges: code - fixing a bug, or adding code logic. doc - writing or improving documentation or docstrings. bug - reporting or diagnosing a bug (get this plus code if you also fixed the bug in the PR).maintenance - CI, test framework, release.
    See here for full badge reference
  • The PR title starts with either [ENH], [MNT], [DOC], or [BUG]. [BUG] - bugfix, [MNT] - CI, test framework, [ENH] - adding or improving code, [DOC] - writing or improving documentation or docstrings.
For new estimators
  • I've added the estimator to the API reference - in docs/source/api_reference/taskname.rst, follow the pattern.
  • I've added one or more illustrative usage examples to the docstring, in a pydocstyle compliant Examples section.
  • If the estimator relies on a soft dependency, I've set the python_dependencies tag and ensured
    dependency isolation, see the estimator dependencies guide.

Copy link
Copy Markdown
Collaborator

@fkiraly fkiraly left a comment

Choose a reason for hiding this comment

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

Somehow your approach causes recursion in the pdf method. I suspect this might be due to how this interacts with _has_implementation_of?

@KaranSinghDev
Copy link
Copy Markdown
Contributor Author

KaranSinghDev commented Apr 5, 2026

@fkiraly Thank you for the observation. I have checked it and I overrode _has_implementation_of in BaseDistribution to unwind the __wrapped__ chain. It now correctly identifies inherited methods even when wrapped for docstring injection.
I have verified locally with a targeted recursion test and the full Test All Distributions suite .

@KaranSinghDev KaranSinghDev requested a review from fkiraly April 5, 2026 15:54
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