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

Update docstrings of distributions logp and logcdf methods. #4859

Closed
ricardoV94 opened this issue Jul 13, 2021 · 5 comments
Closed

Update docstrings of distributions logp and logcdf methods. #4859

ricardoV94 opened this issue Jul 13, 2021 · 5 comments

Comments

@ricardoV94
Copy link
Member

We should update the docstrings of these methods. They are quite verbose, and the same information could be easily conveyed by type hints. More importantly, they are also outdated (e.g., the methods no longer expect only a value as input)

https://github.com/pymc-devs/pymc3/blob/9c7a92481856e1f86c15868b93a65a5d778b0c22/pymc3/distributions/continuous.py#L2198-L2211

@zoj613
Copy link
Contributor

zoj613 commented Jul 14, 2021

Are you suggesting they be removed entirely?

@ricardoV94
Copy link
Member Author

ricardoV94 commented Jul 22, 2021

Are you suggesting they be removed entirely?

Maybe... they are not user facing anymore and all work in the same way. Perhaps it's enough to have the documentation at the parent class level?

What do others suggest?

@zoj613
Copy link
Contributor

zoj613 commented Jul 22, 2021

Are you suggesting they be removed entirely?

Maybe... they are not user facing anymore and all work in the same way. Perhaps it's enough to have the documentation at the parent class level?

What to others suggest?

I do agree. If they don't ever appear in the docs then maybe they can be removed. This will helping scroll easier since the file is getting bigger.

@mjhajharia
Copy link
Member

mjhajharia commented Aug 17, 2021

maybe we can do away with the doctoring and have something like rng_fn for logp, I mean this is enough for explaining the input and output types

def rng_fn(
        cls,
        rng: np.random.RandomState,
        loc: np.ndarray,
        scale: np.ndarray,
        size: Tuple[int, ...],
    ) -> np.ndarray:

@OriolAbril
Copy link
Member

Updating the labels as it's not clear how to go about this or even if this should be done at all.

For some more details see #5282 (comment) and #5308

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

No branches or pull requests

4 participants