search

aimalz / qp

Quantile Parametrization for probability distribution functions module
MIT License
3 stars 9 forks source link

Rethink qp.PDF.limits #95

Open johannct opened 6 years ago

johannct commented 6 years ago

https://github.com/aimalz/qp/blob/7a43a8ffc2298e9714be68e268f15e64856ab877/qp/pdf.py#L89

As limits attribute is provided on top of the grid tuple, I would expect that it means that limits takes over precedence over the grid boundaries. so the lower bound should be the max(grid.min, limits[0]) and the upper bound should be min(grid.max, limits[1])

aimalz commented 6 years ago

The code is written as I originally intended (it's not a "bug" in the sense of an accident), but I think you're right that it probably shouldn't be that way (not being a bug doesn't make it not a mistake). I did think the notion of PDF-wide "limits" was problematic so circumvented it in the upcoming refactored version. I'll leave this issue open as a reminder, though I'm going to rename it for the sake of context. Thanks for your patience with this.