StatisticalRethinking.jl icon indicating copy to clipboard operation
StatisticalRethinking.jl copied to clipboard

PI doc string needs updating

Open eljfe opened this issue 2 years ago • 11 comments

Hi, Thanks for this package!

Small issue with the doc string. The doc string proposes the prob= argument, where it should be perc_prob=

To reproduce, ... way down at the bottom is the PI(1:10; prob=0.1) argument.

help?> PI
search: plot_density_interval PI pi Pipe pie pie! pipeline PipeBuffer sinpi cospi cispi getpid rem2pi mod2pi groupindices Pair print

  PI
  ≡≡≡≡

  Compute percentile central interval of data. Returns vector of bounds.

  PI(data; perc_prob)
  

  Required arguments
  ====================

    •  data: iterable over data values

  Optional arguments
  ====================

    •  prob::Float64=0.89: percentile interval to calculate

  Examples
  ==========

  julia> using StatisticalRethinking
  
  julia> PI(1:10)
  2-element Vector{Float64}:
   1.495
   9.505
  
  julia> PI(1:10; prob=0.1)
  2-element Vector{Float64}:
   5.05
   5.95

It got me at first.

eljfe avatar Jun 16 '23 21:06 eljfe

@eljfe Hi Jeffrey, thanks a lot. I’m currently on a trip without my laptop. Should be back home later this week (Thursday) and test and apply your proposed changes! Thanks again, Rob

goedman avatar Jun 18 '23 13:06 goedman

@goedman. Hi Rob. I want to offer to do it. It's a really trivial change. That said, I've never contributed to any open source projects. It might be more trouble than it's worth getting me sorted on git. I have my own personal repos but never done a PR. Anyhow, let me know if you would like my input...

Enjoy your trip without a laptop ;-)

JEff

eljfe avatar Jun 19 '23 00:06 eljfe

If you fork the repo, you can make the changes, test it (indeed trivial in this case) and create a PR (pull request). I will then merge your pull request.

Rob

goedman avatar Jun 19 '23 01:06 goedman

Send PR a second ago. I think it went off anyhow...

eljfe avatar Jun 19 '23 03:06 eljfe

Hi @eljfe ,

Haven’t seen the PR yet. I’m assuming you forked the repo to your Github account, installed/pulled the fork on your machine, activated it as a Julia project and tested your update. Once that works, did you push it back to your Github fork? And from there created a PR on the parent repo? A bit hard to really help out here without my laptop and we’re also probably using different tools.

Rob

goedman avatar Jun 19 '23 13:06 goedman

Looks like I've managed to get the PR right sent properly. I see it in the repo's Pull Requests queue.
Thx Rob

eljfe avatar Jun 19 '23 23:06 eljfe

Hi @eljfe The release (v4.7.1) should now be available! Thanks for you help!

goedman avatar Jun 22 '23 15:06 goedman

@goedman Thanks to you. Appreciate your patience. My first open source PR... kinda fun. 😀

eljfe avatar Jun 23 '23 13:06 eljfe

@eljfe

As I work only with Stan (not Turing) merging and testing of both packages is a bit less fun for non-Stan users.

I really need to put a lot more time in StatisticalRethinking.jl, in particular switching to Julia Extensions.

For my own work, mainly in the project SR2StanPluto.jl, I have switched to RegressionAndOtherStories.jl which covers similar project support functions.

I'll close this issue in a day or 3.

Again, thanks for you help, Rob.

goedman avatar Jun 23 '23 13:06 goedman

@goedman Let me know if you need any help. I can't say I have a lot of time myself, but I would like to support the project. Using it now for self learning, and finding it useful.

I must confess I had no idea what Julia Extensions were until now. Your mileage may vary 😄

Jeff

eljfe avatar Jun 24 '23 15:06 eljfe

Hi @eljfe ,

Will do. I need to do some planning for this work as it involves multiple layers (project level, on top of StatisticalRethinkingJulia packages (and ROS), on top of StanJulia packages).

At the moment I’m mainly working on replacing my own package StructuralCausalModels.jl by CausalInference.jl. The DAG stuff is really what I’m after.

Rob

goedman avatar Jun 25 '23 13:06 goedman