interpolated plot

[mjhajharia]

Added plot for Interpolated continuous distribution #3859 @canyon289 @OriolAbril

[canyon289]

Dont have time for full review at just this moment (will later today) but wanted to say at a glance this is looking great! Will be much easier for us to help you precisely now and get this going!

Looks like there's a precommit failure. I would check out that error message and see what its complaining about so you know what to fix

[OriolAbril]

Thanks for the PR! Will you want to update the styles of the other plots here too? Or would you prefer opening a different PR for that?

[mjhajharia]

thanks! i'll get back to these comments soon, also I feel like opening a new PR for that would be better?

…

On Mar 22, 2021, 9:21 PM +0530, Oriol Abril-Pla ***@***.***>, wrote: @OriolAbril commented on this pull request. Thanks for the PR! Will you want to update the styles of the other plots here too? Or would you prefer opening a different PR for that? In pymc3/distributions/continuous.py: > @@ -4226,6 +4226,31 @@ class Interpolated(BoundedContinuous): ======== =========================================== Support :math:`x \in [x\_points[0], x\_points[-1]]` ======== =========================================== + The example should go above the support thinghy and inside a .. plot:: directive. You can see one example of this at: https://github.com/pymc-devs/pymc3/blob/master/pymc3/distributions/continuous.py#L193 In pymc3/distributions/continuous.py: > @@ -4226,6 +4226,31 @@ class Interpolated(BoundedContinuous): ======== =========================================== Support :math:`x \in [x\_points[0], x\_points[-1]]` ======== =========================================== + + import matplotlib.pyplot as plt + import numpy as np + import pymc3 as pm + import warnings + warnings.simplefilter(action="ignore", category=FutureWarning) This should not be used, if the code raises any warning we should address it instead of ignoring it. If you share the warning message we can help fix it. In pymc3/distributions/continuous.py: > @@ -4226,6 +4226,31 @@ class Interpolated(BoundedContinuous): ======== =========================================== Support :math:`x \in [x\_points[0], x\_points[-1]]` ======== =========================================== + + import matplotlib.pyplot as plt + import numpy as np + import pymc3 as pm + import warnings + warnings.simplefilter(action="ignore", category=FutureWarning) + from scipy.stats import gamma + a = 1.99 + mean, var, skew, kurt = gamma.stats(a, moments='mvsk') I don't think these are needed. In pymc3/distributions/continuous.py: > @@ -4226,6 +4226,31 @@ class Interpolated(BoundedContinuous): ======== =========================================== Support :math:`x \in [x\_points[0], x\_points[-1]]` ======== =========================================== + + import matplotlib.pyplot as plt + import numpy as np + import pymc3 as pm We should also import arviz to be safe. ArviZ has its own styles, but they are only made available to matplotlib when arviz is imported, the code would work if arviz has already been imported at some point during the doc generation (which is quite probable) but we should not rely on that. In pymc3/distributions/continuous.py: > + plt.style.use('arviz-darkgrid') + plt.style.use('tableau-colorblind10') + #plt.set_cmap('coolwarm') ⬇️ Suggested change - plt.style.use('arviz-darkgrid') - plt.style.use('tableau-colorblind10') - #plt.set_cmap('coolwarm') + plt.style.use('arviz-darkgrid') Following on the last issue comment, only the import and this line are needed to use arviz-darkgrid style, there is no need for inferencedata or any other arviz related thing. We may not be very good at advertising it, but ArviZ has many cool features in addition to inferencedata and functions that take inferencedata as input. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

[canyon289]

@almostmeenal A new PR to update the other plots would be best, both to allow you to merge this pr quickly and so we can help you more easily with a small focus

[mjhajharia]

Dont have time for full review at just this moment (will later today) but wanted to say at a glance this is looking great! Will be much easier for us to help you precisely now and get this going!

Looks like there's a precommit failure. I would check out that error message and see what its complaining about so you know what to fix

yeah that was an editor white space issue it isnt there in the second commit

[canyon289]

This looks great. Will merge after CI pass

[canyon289]

Great job @almostmeenal. Just so yo know your commit messages are informative and nice to read as well. Thanks for putting thought into them