Updates to pymc.Hypergeometric docstrings

[cuchoi]

As part of the work of #5459:

• Added the bounds to the parameters N, k, and n in the parameters section of pymc.Hypergeometric.
• Added docstrings for parameters "good" and "bad" in logp and logcdf. These parameters are different than the ones used in __init__, which is odd to me. They transform the N and k parameters to bad (which is N - k) and good (which is k), but this transformation could happen inside logp and logcdf. Would it make sense to always use N, k?

#DataUmbrellaPyMCSprint

[OriolAbril]

are scalars the only valid type for all 3 arguments? cc @ricardoV94 ? (no idea who knows, feel free to tag someone else)

[cuchoi]

They should be only integers: https://en.wikipedia.org/wiki/Hypergeometric_distribution

[OriolAbril]

but maybe an array of integers is also supported? 🤔

[cuchoi]

logcdf does say:

        # logcdf can only handle scalar values at the moment
        if np.ndim(value):
            raise TypeError(
                f"HyperGeometric.logcdf expects a scalar value but received a {np.ndim(value)}-dimensional object."
            )

[cuchoi]

But maybe logp supports it?

[cuchoi]

Can I use a different format to specify the array types? Example integer, array_like[integer] or TensorVariable[integer]?

[OriolAbril]

no, numpydoc formatting is very clear in the array of type format

[OriolAbril]

We decided to use tensor_like of integer and we'll add its definition on the glossary so we have a type that covers scalars, arrays, aesara tensorvariable and randomvariable

[cuchoi]

Noted! Picked integer, array_like or TensorVariable

[OriolAbril]

Sorry for the late review. We decided to go with tensor_like of integer which will get rendered as a link to our glossary where we have an explanation on what inputs are accepted for arguments documented like this: https://docs.pymc.io/en/latest/glossary.html#term-tensor_like

[OriolAbril]

@cuchoi did you mean to close the PR? maybe open another not from main?

[cuchoi]

I closed the PR while adding more changes to it to avoid it being reviewed before adding them.

Should I change the PR to be from a branch of my forked repo?

[cuchoi]

Added docstrings for parameters "good" and "bad" in logp and logcdf. These parameters are different than the ones used in init, which is odd to me. They transform the N and k parameters to bad (which is N - k) and good (which is k), but this transformation could happen inside logp and logcdf. Would it make sense to always use N, k?

[cuchoi]

Should we also use "tensor_like of integer" here @OriolAbril ?

[OriolAbril]

We are in the process of removing the distribution methods from the documentation and leaving only the dist one. The Distribution class is still being refactored, see #5308 for some more details (I don't really understand what is going on very well) and those methods might be removed completely, and because they should not be used themselves already but called via pm.logp() or pm.logcdf

I think it's best to leave them be for now.