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

Issue 169: Natural scale postprocessing #170

Merged
merged 24 commits into from
Jul 29, 2024
Merged

Issue 169: Natural scale postprocessing #170

merged 24 commits into from
Jul 29, 2024

Conversation

athowes
Copy link
Collaborator

@athowes athowes commented Jul 16, 2024

Description

This draft PR will close #169.

Checklist

  • My PR is based on a package issue and I have explicitly linked it.
  • I have included the target issue or issues in the PR title in the for Issue(s) issue-numbers: PR title
  • I have read the contribution guidelines.
  • I have tested my changes locally.
  • I have added or updated unit tests where necessary.
  • I have updated the documentation if required.
  • My code follows the established coding standards.
  • I have added a news item linked to this PR.
  • I have reviewed CI checks for this PR and addressed them as far as I am able.

R/defaults.R Show resolved Hide resolved
@athowes
Copy link
Collaborator Author

athowes commented Jul 22, 2024

Created new issue for follow-up a function to do strata-level delay mean and sd: #188

In future will have issues with more complexity about the other features we might want to support like posterior predictive, input of other data to the strata.

@athowes athowes requested a review from seabbs July 23, 2024 10:36
@athowes athowes marked this pull request as ready for review July 23, 2024 10:36
@athowes athowes requested a review from kgostic July 23, 2024 11:06
R/postprocess.R Outdated Show resolved Hide resolved
Copy link
Contributor

@seabbs seabbs left a comment

Choose a reason for hiding this comment

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

confirming that waiting on being repinged for review after our f2f.

@athowes
Copy link
Collaborator Author

athowes commented Jul 26, 2024

Waiting for one test to finish running but I think we should be good to review this now @seabbs.

@athowes athowes requested a review from seabbs July 26, 2024 10:30
@seabbs seabbs enabled auto-merge (squash) July 29, 2024 11:50
R/defaults.R Show resolved Hide resolved
seabbs
seabbs previously approved these changes Jul 29, 2024
Copy link
Contributor

@seabbs seabbs left a comment

Choose a reason for hiding this comment

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

Some comments about issues connected to but not part of add_mean_sd. Suggest deal with in future issues/PRs

R/fitting-and-postprocessing.R Show resolved Hide resolved
R/fitting-and-postprocessing.R Show resolved Hide resolved
@seabbs seabbs disabled auto-merge July 29, 2024 12:22
@seabbs seabbs merged commit 9757409 into main Jul 29, 2024
8 checks passed
@seabbs seabbs deleted the natural-scale branch July 29, 2024 12:22
@github-actions github-actions bot restored the natural-scale branch July 29, 2024 12:23
@athowes athowes deleted the natural-scale branch July 29, 2024 12:58
@athowes athowes mentioned this pull request Aug 9, 2024
3 tasks
seabbs added a commit that referenced this pull request Jan 10, 2025
* Remove standard function just for lognormal

* Basic structure for add_natural_scale_mean_sd as generic

* Building this out a bit. Add family to class. Start test script

* Get link functions (but they're names, how to get functions)

* Finding a barrier here about using brms::predict

* Use same distributional parameter names as brms

* Better documentation of data object

* posterior_linpred doesn't error!

* Add mean and sd for gamma

* Perhaps prepare_predictions is useful?

* Melt them down into long

* Call to document

* Format into start of tests, correct some gamma issues, use the class "family_samples" as placeholder for the time being

* Add plausible tests

* Linting!

* Add simulation test and skip fitting test on CRAN

* Do this without using reshape2 (jankily)

* Making the package and vignettes (awkwardly!) compatible with changes to add_natural_scale_mean_sd

* Alter tests to pass

* Replace add_natural_scale_mean_sd by add_mean_sd

* lp not ld

* Missed one function call, and document()

* Test correction

---------

Co-authored-by: Sam Abbott <[email protected]>
Former-commit-id: 917a141ddc1a7ca8b8efe1582c533d7c21230dc7 [formerly a1ab87144e42ed6e9118a2538eb242508658b95f]
Former-commit-id: e98a329a7678bfcadbfea2386a7618e83f755832
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.

Solution for natural scale outputs
2 participants