StatisticalRethinkingJulia / StatisticalRethinking.jl

Julia package with selected functions in the R package `rethinking`. Used in the SR2... projects.
MIT License
385 stars 32 forks source link

PI doc string needs updating #160

Open eljfe opened 1 year ago

eljfe commented 1 year ago

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.

goedman commented 1 year ago

@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

eljfe commented 1 year ago

@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

goedman commented 1 year ago

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

eljfe commented 1 year ago

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

goedman commented 1 year ago

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

eljfe commented 1 year ago

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

goedman commented 1 year ago

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

eljfe commented 1 year ago

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

goedman commented 1 year ago

@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.

eljfe commented 1 year ago

@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

goedman commented 1 year ago

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