Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PI doc string needs updating #160

Open
eljfe opened this issue Jun 16, 2023 · 11 comments
Open

PI doc string needs updating #160

eljfe opened this issue Jun 16, 2023 · 11 comments

Comments

@eljfe
Copy link
Contributor

eljfe commented Jun 16, 2023

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
Copy link
Member

goedman commented Jun 18, 2023

@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
Copy link
Contributor Author

eljfe commented Jun 19, 2023

@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
Copy link
Member

goedman commented Jun 19, 2023

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
Copy link
Contributor Author

eljfe commented Jun 19, 2023

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

@goedman
Copy link
Member

goedman commented Jun 19, 2023

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
Copy link
Contributor Author

eljfe commented Jun 19, 2023

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
Copy link
Member

goedman commented Jun 22, 2023

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

@eljfe
Copy link
Contributor Author

eljfe commented Jun 23, 2023

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

@goedman
Copy link
Member

goedman commented Jun 23, 2023

@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
Copy link
Contributor Author

eljfe commented Jun 24, 2023

@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
Copy link
Member

goedman commented Jun 25, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants