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.

Sign up for GitHub

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

Jump to bottom

Ensure euc yi vars not erroneously being back-transformed #88

Closed
1 task
egouldo opened this issue Aug 5, 2024 · 4 comments · Fixed by #112
Closed
1 task

Ensure euc yi vars not erroneously being back-transformed #88

egouldo opened this issue Aug 5, 2024 · 4 comments · Fixed by #112
Assignees
@egouldo
Labels
bug an unexpected problem or unintended behavior reprex needs a minimal reproducible example
Milestone
Respond Reviewer ...

Comments

@egouldo
Copy link
Owner

egouldo commented Aug 5, 2024

Check that yi vars are not being back-transformed / standardised when they shouldn't be for Eucalyptus, which is what we explained in the manuscript.

Related to #80 #82

Note that this has implications for the manuscript:

  • ensure that plots for euc variables don't repeat back-transformations unnecessarily (meta-analysis should already be on stem-count scale, i.e. not standardised as per Z_VZ_preds()), however, we should continue logging the outcome variable.
@egouldo egouldo added bug an unexpected problem or unintended behavior question labels Aug 5, 2024
@egouldo egouldo added this to the Respond Reviewer Comments milestone Aug 5, 2024
@egouldo egouldo self-assigned this Aug 5, 2024
@egouldo
Copy link
Owner Author

egouldo commented Aug 5, 2024

Note, related to: egouldo/ManyAnalysts#63

@egouldo egouldo added reprex needs a minimal reproducible example and removed question labels Aug 8, 2024
@egouldo
Copy link
Owner Author

egouldo commented Aug 10, 2024

See also: https://github.com/metamelb-repliCATS/ManyAnalysts/issues/194#issuecomment-1624523475 we said we would log-transform them.

@egouldo
Copy link
Owner Author

egouldo commented Aug 12, 2024

OK yes, unfortunately it seems that the Euc variables are both back-transformed AND standardised, which explains why we were getting weird forest plots for the Eucs and had to log-transform the response... https://github.com/metamelb-repliCATS/ManyAnalysts/issues/194#issuecomment-1624523475 !!

@egouldo egouldo mentioned this issue Aug 12, 2024
Closed
egouldo added a commit that referenced this issue Aug 12, 2024
@egouldo
Fix #104 reorganise yi computational pipeline in prep for #88
5a101e2 
- #104 mv `assign_transformation_type()` and `convert_predictions()` out of `standardise_response()` into `back_transform_response_vars_yi()`
- Call `back_transform_response_vars_yi()` in `prepare_response_variables()` for yi workflow (was not being called currently) #104
- #104 keep addition of `param_table` inside `standardise_response()`
@egouldo egouldo mentioned this issue Aug 12, 2024
Closed
1 task
egouldo added a commit that referenced this issue Aug 14, 2024
@egouldo
#88 add argument for selectively standardising
a75be28 
- NOTE: dummy function for datasets not selected for standardising is currently applied
- TODO: update dummy function
- TODO: apply functions with map2
egouldo added a commit that referenced this issue Aug 14, 2024
@egouldo
#102 add @imports call and document new param for new arg in `prepa…
c58621d 
…re_response_variables()` #88
egouldo added a commit that referenced this issue Aug 14, 2024
@egouldo
#88 switch to alternative approach for assigning fns to list-col
7f7cc63 
- prev approach with rlang::as_function() was taking the entire set of all previous input code including the wrapping mutate() calls
egouldo added a commit that referenced this issue Aug 14, 2024
@egouldo
#88 conditionally apply standardisation
e22ff1b 
- correctly apply `fns` in list-col using `pmap()` approach on entire dataframe, with support of wrapper / parser fun that parses the required variables to either `standardise_response()` or `process_response()`
- #97 delete comment
egouldo added a commit that referenced this issue Aug 14, 2024
@egouldo
#88 add function arg checks
86ec431 
#97 delete comment
#97 lint
egouldo added a commit that referenced this issue Aug 14, 2024
@egouldo
#88 add process_response()
e07d131 
- for alternate to standardise_response()
egouldo added a commit that referenced this issue Aug 14, 2024
@egouldo
#88 rm unused args in process_response() and replace with ...
4d556d0 
- #102 Add `@details` section for `process_response()` and add heading for `@details` in `standardise_response()`
egouldo added a commit that referenced this issue Aug 14, 2024
@egouldo
close #88 update function call to prepare_response_variables() in t…
1612659 
…argets

- only standardise blue tit yi #88
egouldo added a commit that referenced this issue Aug 14, 2024
@egouldo
devtools::document() #97 #102 #88
b381581
egouldo added a commit that referenced this issue Aug 14, 2024
@egouldo
#88 tar_make.R error - rm left over line from #82
8b438cb
egouldo added a commit that referenced this issue Aug 14, 2024
@egouldo
Fix #88 rebuild targets
d143734
This was referenced Aug 14, 2024
Closed
Merged
egouldo added a commit that referenced this issue Aug 14, 2024
@egouldo
Merge pull request #112 from egouldo/88-no-euc-standardisation
42b1e38 
Fix #88 conditionally apply standardisation to datasets
egouldo added a commit that referenced this issue Aug 15, 2024
@egouldo
Update function prepare_response_variables()
72bdaf5 
- #116 add checks for param table argument
- update default behaviour description #102
- line linting / indentation #97
- #88 change default behaviour to `process_response()`, not `standardise_response()` when `is.null(dataset_standardise)`, otherwise function did not allow for case when all datasets should not be Z-standardised
@egouldo egouldo mentioned this issue Aug 15, 2024
Merged
@egouldo
Copy link
Owner Author

egouldo commented Aug 15, 2024 •
edited
Loading

  • update downstream processing to log-transform out-of-sample euc estimates

@egouldo egouldo mentioned this issue Aug 15, 2024
Closed
egouldo added a commit that referenced this issue Aug 23, 2024
@egouldo
#88 fix: ensure log_transform also calculates SE and not SD
ea032bc
@egouldo egouldo mentioned this issue Aug 28, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@egouldo egouldo

Labels
bug an unexpected problem or unintended behavior reprex needs a minimal reproducible example
Projects
None yet
Milestone
Respond Reviewer Comments
Development

Successfully merging a pull request may close this issue.

Fix #88 conditionally apply standardisation to datasets egouldo/ManyEcoEvo
1 participant
@egouldo