Rename pm.Constant to pm.DiracDelta #5903
Conversation
Codecov Report
@@ Coverage Diff @@
## main #5903 +/- ##
==========================================
- Coverage 89.50% 89.49% -0.01%
==========================================
Files 73 73
Lines 13267 13275 +8
==========================================
+ Hits 11874 11880 +6
- Misses 1393 1395 +2
|
|
I think we should deprecate. |
What do you mean, have an alias with a warning for a while? |
|
Yeah. |
I agree |
Let's not add it to the new renamed distribution, hopefully the name is good enough to dissuade people. We could add a warning message in docstrings. For the deprecation, you will have to do something like: class Constant:
def __new__(cls, *args, **kwargs):
warnings.warn("bla bla blah", FutureWarning)
return DiracDelta(*args, **kwargs)
@classmethod
def dist(cls, *args, **kwargs):
warnings.warn("blah blah blah", FutureWarning)
return DiracDelat.dist(*args, **kwargs) |
|
API docs at https://github.com/pymc-devs/pymc/blob/main/docs/source/api/distributions/discrete.rst also need to be updated |
Should the now-deprecated pm.Constant be documented at all or should it be hidden to (further) discourage its use? |
Hidden I think |
pymc/tests/test_distributions.py
Outdated
| def test_constantdist(self): | ||
| check_logp(Constant, I, {"c": I}, lambda value, c: np.log(c == value)) | ||
| check_logcdf(Constant, I, {"c": I}, lambda value, c: np.log(value >= c)) | ||
| with pytest.warns(FutureWarning): |
There was a problem hiding this comment.
| with pytest.warns(FutureWarning): | |
| with pytest.warns(FutureWarning, match="..."): |
There was a problem hiding this comment.
This test can be at the end of the file, it shouldn't be in this generic class that checks for logp/logcdf stuff
|
Hehe, I'm on the train and was just reading through the updates here, wanting to review.. |
Fixes #5716, renaming pm.Constant as pm.DiractDelta. This is currently a basic find-and-replace in both the source and tests. In the original issue, there was mention of adding a user warning about use of this distribution (i.e., sampling is hard). However, pm.Constant had no
__new__method, so I wasn't sure where to add it. Happy to do so once I know where.