Skip to content

Conversation

avehtari
Copy link
Member

Since we do use posterior package elsewhere,
replace gpdfit() and qgpd() with posterior::gpdfit and posterior::qgeneralized_pareto()

This will make maintenance easier. For example, there is a PR for posterior::gpdfit() to make it more robust and posterior::qgeneralized_pareto() already is more robust than qgpd()

This is related to #249, but timing is due to the fix PR for posterior::gpdfit()

@avehtari avehtari requested a review from jgabry October 17, 2025 17:10
Copy link
Member

@jgabry jgabry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the R cmd check error is because the NAMESPACE file is still trying to export gpdfit from loo. If you run devtools::document() it should fix this and then we can see if all the tests pass.

@avehtari
Copy link
Member Author

If it has been exported, then someone might be using it? Should I put it back and define it with posterior::qgeneralized_pareto()?

@avehtari
Copy link
Member Author

At least in github only I and Juho Timonen have used loo::gpdfit, so I guess it can be removed

@jgabry
Copy link
Member

jgabry commented Oct 17, 2025

Good point. Yeah you can just replace the bodies of the functions with calls to the posterior versions.

@jgabry
Copy link
Member

jgabry commented Oct 17, 2025

At least in github only I and Juho Timonen have used loo::gpdfit, so I guess it can be removed

OK in that case, we can try removing it and we will find out in the reverse dependency checks whether any packages are using it

@jgabry
Copy link
Member

jgabry commented Oct 17, 2025

The error now is because apparently these functions aren't exported from posterior, they're just internal functions. That would be easy to change but it would require doing a CRAN release of posterior.

@avehtari
Copy link
Member Author

my bad, I guess this needs to wait for the next posterior release

@jgabry
Copy link
Member

jgabry commented Oct 17, 2025

There is also #290 from @VisruthSK that is waiting to be merged until there is a new release of posterior

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

Successfully merging this pull request may close these issues.

2 participants