Substitute nested `ifelse` with `data.table::fcase()` #383

jamesmbaazam · 2023-05-04T15:43:14Z

This PR replaces all occurrences of nested ifelse with data.table::fcase() and fixes #379. Each commit mentions the function that was changed, making it easier to roll back to in case of any consequences of the change.

github-actions · 2023-05-04T21:11:10Z

This is how benchmark results would change (along with a 95% confidence interval in relative change) if 82053ed is merged into main:

:ballot_box_with_check:default: 1.11m -> 1.08m [-19.08%, +13.1%]
:ballot_box_with_check:no_delays: 52.6s -> 54.6s [-7.39%, +14.9%]
:ballot_box_with_check:random_walk: 17.1s -> 16.5s [-16.75%, +10%]
:ballot_box_with_check:stationary: 43.4s -> 41.5s [-14.16%, +5.18%]
:ballot_box_with_check:uncertain: 1.03m -> 1.09m [-5.62%, +17.04%]
Further explanation regarding interpretation and methodology can be found in the documentation.

github-actions · 2023-05-05T09:36:26Z

This is how benchmark results would change (along with a 95% confidence interval in relative change) if bf4080d is merged into main:

:ballot_box_with_check:default: 45.2s -> 44.7s [-16.09%, +13.99%]
:ballot_box_with_check:no_delays: 39.1s -> 36.1s [-20.14%, +4.82%]
:ballot_box_with_check:random_walk: 12.9s -> 12.5s [-7.93%, +1.22%]
:ballot_box_with_check:stationary: 30s -> 30.6s [-6.5%, +10.99%]
:ballot_box_with_check:uncertain: 46.3s -> 49.1s [-4.9%, +16.89%]
Further explanation regarding interpretation and methodology can be found in the documentation.

…ata()

R/dist.R

…oper comment tags in examples

seabbs · 2023-05-09T13:11:26Z

Are you waiting for review on this @jamesmbaazam? Before I can review it would ideally be passing CI checks or there would be some detail on why it isn't and why that is okay

jamesmbaazam · 2023-05-09T13:45:27Z

No, I'm not waiting. I am about to try and fix the checks.

…message" This reverts commit 7b63d9b.

jamesmbaazam · 2023-05-09T17:14:06Z

No success so far! The problem seems to come from somewhere within epinow() but not exactly where. I will keep looking!

R/dist.R

R/create.R

R/estimate_infections.R

R/create.R

jamesmbaazam · 2023-05-11T17:55:54Z

I've incremented the dev version and added a news item here 276af42.

github-actions · 2023-05-11T19:42:18Z

This is how benchmark results would change (along with a 95% confidence interval in relative change) if a7c8f1f is merged into main:

:ballot_box_with_check:default: 48.9s -> 46.9s [-19.37%, +11.22%]
:ballot_box_with_check:no_delays: 38.2s -> 37.3s [-12.19%, +7.56%]
:ballot_box_with_check:random_walk: 12.9s -> 12.9s [-8.93%, +8.85%]
:ballot_box_with_check:stationary: 32.2s -> 30.4s [-13.92%, +2.85%]
:ballot_box_with_check:uncertain: 46.6s -> 48s [-9.34%, +15.56%]
Further explanation regarding interpretation and methodology can be found in the documentation.

github-actions · 2023-05-12T17:36:08Z

This is how benchmark results would change (along with a 95% confidence interval in relative change) if ddcafca is merged into main:

:ballot_box_with_check:default: 48.2s -> 48s [-17.46%, +16.8%]
:ballot_box_with_check:no_delays: 38.8s -> 38.2s [-14.92%, +11.82%]
:ballot_box_with_check:random_walk: 11.6s -> 11.9s [-6.02%, +11.54%]
:ballot_box_with_check:stationary: 31.5s -> 29.7s [-23.76%, +12.49%]
:ballot_box_with_check:uncertain: 50.2s -> 49.3s [-17.34%, +13.92%]
Further explanation regarding interpretation and methodology can be found in the documentation.

github-actions · 2023-05-16T01:18:03Z

This is how benchmark results would change (along with a 95% confidence interval in relative change) if 491ba64 is merged into main:

:ballot_box_with_check:default: 1.14m -> 1.08m [-14.22%, +3.33%]
:ballot_box_with_check:no_delays: 54.6s -> 51.5s [-26.97%, +15.78%]
:ballot_box_with_check:random_walk: 18s -> 17.4s [-16.94%, +11.1%]
:ballot_box_with_check:stationary: 41.8s -> 42.6s [-7.77%, +11.72%]
:rocket:uncertain: 1.25m -> 1.06m [-24.75%, -5.54%]
Further explanation regarding interpretation and methodology can be found in the documentation.

seabbs

Very nearly. Just a few style, detail, and infra quirks to update.

DESCRIPTION

seabbs · 2023-05-16T07:33:36Z

NEWS.md

-# EpiNow2 1.3.6.2000
+# EpiNow2 (development version)
+
+# EpiNow2 1.3.6.2001


This can be just epinow2 1.3.6 it doesn't need the nested title

This has been fixed in 37786f0.

NEWS.md

R/create.R

R/dist.R

R/estimate_infections.R

github-actions · 2023-05-17T00:22:24Z

This is how benchmark results would change (along with a 95% confidence interval in relative change) if 22c387b is merged into main:

:ballot_box_with_check:default: 1.1m -> 59.4s [-21.3%, +0.95%]
:ballot_box_with_check:no_delays: 54.5s -> 51.6s [-15.97%, +5.27%]
:ballot_box_with_check:random_walk: 16.4s -> 17.1s [-8.08%, +15.64%]
:ballot_box_with_check:stationary: 40.7s -> 40s [-9.67%, +6.61%]
:ballot_box_with_check:uncertain: 1.15m -> 1.04m [-24.52%, +4.58%]
Further explanation regarding interpretation and methodology can be found in the documentation.

R/create.R

github-actions · 2023-05-17T10:10:04Z

This is how benchmark results would change (along with a 95% confidence interval in relative change) if 7274c21 is merged into main:

:ballot_box_with_check:default: 44.7s -> 45.9s [-5.27%, +10.52%]
:ballot_box_with_check:no_delays: 38.2s -> 35.5s [-30.86%, +16.92%]
:ballot_box_with_check:random_walk: 19.6s -> 11.7s [-132.04%, +51.38%]
❗🐌stationary: 28.6s -> 32.1s [+7.11%, +17.4%]
:ballot_box_with_check:uncertain: 43.1s -> 43.2s [-18.28%, +18.8%]
Further explanation regarding interpretation and methodology can be found in the documentation.

seabbs

LGTM to me - thanks James.

Maybe worth setting up a call between you, seb, and me to debrief how you felt about this and things we could do better in the review process.

Just waiting for checks to pass and then will merge.

seabbs

LGTM to me - thanks James.

Maybe worth setting up a call between you, seb, and me to debrief how you felt about this and things we could do better in the review process.

Just waiting for checks to pass and then will merge.

jamesmbaazam · 2023-05-17T12:00:07Z

Thanks for reviewing, Sam.

I agreed that we should set up a call.

My schedule is usually open on Mondays, Tuesdays, and Fridays.

seabbs · 2023-05-17T12:48:06Z

Friday morning this week or Monday morning works for me.

github-actions · 2023-05-17T13:04:48Z

This is how benchmark results would change (along with a 95% confidence interval in relative change) if 83ad88a is merged into main:

:ballot_box_with_check:default: 1.05m -> 1.06m [-11.62%, +13.65%]
:ballot_box_with_check:no_delays: 47.4s -> 50.1s [-6.74%, +18.13%]
:ballot_box_with_check:random_walk: 16.2s -> 16.4s [-8.77%, +11.55%]
:ballot_box_with_check:stationary: 39.5s -> 38s [-10.28%, +2.88%]
:ballot_box_with_check:uncertain: 1.03m -> 1.03m [-11.32%, +9.67%]
Further explanation regarding interpretation and methodology can be found in the documentation.

* updated gitignore * replaced ifelse with fcase in map_prob_change() * revised explicit breakpoints for the categories * replace nested ifelse() with data.table::fcase() in create_backcalc_data() * set the default samples argument to 1000 and added a warning message * replaced ifelse with data.table::fcase in create_gp_data() * replaced ifelse with fcase in create_initial_conditions() * moved warning message into rightful place (inside the if() block) * replaced ifelse with data.table::fcase in regional_summary() * replaced ifelse with data.table::fcase in format_fit() * linting: removed nolint tags and unnecessary whitespace, and added proper comment tags in examples * Revert "set the default samples argument to 1000 and added a warning message" This reverts commit 7b63d9b. * register fcase import * redoc create_backcalc_data * make rho an array * linting: remove whitespace * added a condition for when date is NA * removed linting gates * fixed indentation * added James Azam as contributor * removed a space * incremented the version and added a news item * removed a space * linting: removed whitespace and added lint gates * redoc'd package * incremented dev version * changed package version in NEWS * Updated news entry to link PR * fixed indentation * remove space * fix space --------- Co-authored-by: Sam Abbott <contact@samabbott.co.uk>

jamesmbaazam added 2 commits May 4, 2023 16:39

updated gitignore

0023aba

replaced ifelse with fcase in map_prob_change()

6d1eccc

jamesmbaazam changed the title ~~Substiture occurrences of ifelse with data.table::fcase()~~ Substitute occurrences of ifelse with data.table::fcase() May 4, 2023

revised explicit breakpoints for the categories

7e91528

jamesmbaazam added 6 commits May 5, 2023 13:58

replace nested ifelse() with data.table::fcase() in create_backcalc_d…

c8db992

…ata()

set the default samples argument to 1000 and added a warning message

7b63d9b

replaced ifelse with data.table::fcase in create_gp_data()

6a8a115

replaced ifelse with fcase in create_initial_conditions()

e27fe70

moved warning message into rightful place (inside the if() block)

ba2572d

replaced ifelse with data.table::fcase in regional_summary()

9274e7a

seabbs reviewed May 5, 2023

View reviewed changes

R/dist.R Outdated Show resolved Hide resolved

jamesmbaazam added 2 commits May 5, 2023 17:07

replaced ifelse with data.table::fcase in format_fit()

188a79a

linting: removed nolint tags and unnecessary whitespace, and added pr…

72e04de

…oper comment tags in examples

jamesmbaazam marked this pull request as ready for review May 5, 2023 16:44

jamesmbaazam changed the title ~~Substitute occurrences of ifelse with data.table::fcase()~~ Substitute nested ifelse with data.table::fcase() May 5, 2023

Revert "set the default samples argument to 1000 and added a warning …

b3ced53

…message" This reverts commit 7b63d9b.

seabbs reviewed May 9, 2023

View reviewed changes

R/dist.R Outdated Show resolved Hide resolved

jamesmbaazam added 2 commits May 9, 2023 21:24

register fcase import

f2a2a36

redoc create_backcalc_data

ff9412c

sbfnk reviewed May 9, 2023

View reviewed changes

R/create.R Outdated Show resolved Hide resolved

jamesmbaazam added 2 commits May 9, 2023 22:11

make rho an array

f8a5cb2

linting: remove whitespace

0d79c79

sbfnk reviewed May 10, 2023

View reviewed changes

R/estimate_infections.R Outdated Show resolved Hide resolved

added a condition for when date is NA

a173a03

seabbs enabled auto-merge May 10, 2023 18:25

fixed indentation

84491c8

seabbs reviewed May 11, 2023

View reviewed changes

R/create.R Outdated Show resolved Hide resolved

jamesmbaazam added 3 commits May 11, 2023 18:42

added James Azam as contributor

610fb6d

removed a space

bf9459c

incremented the version and added a news item

276af42

removed a space

3841eaa

jamesmbaazam requested a review from seabbs May 12, 2023 15:07

jamesmbaazam added 2 commits May 13, 2023 16:33

linting: removed whitespace and added lint gates

6109f24

redoc'd package

74f699b

seabbs requested changes May 16, 2023

View reviewed changes

jamesmbaazam added 4 commits May 16, 2023 21:24

incremented dev version

b853c98

changed package version in NEWS

37786f0

Updated news entry to link PR

b286eba

fixed indentation

e883619

seabbs reviewed May 17, 2023

View reviewed changes

R/create.R Outdated Show resolved Hide resolved

seabbs added 2 commits May 17, 2023 11:34

remove space

70483ea

fix space

2f2a765

seabbs approved these changes May 17, 2023

View reviewed changes

seabbs merged commit 3a619d0 into epiforecasts:main May 17, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Substitute nested `ifelse` with `data.table::fcase()` #383

Substitute nested `ifelse` with `data.table::fcase()` #383

jamesmbaazam commented May 4, 2023 •

edited

Loading

github-actions bot commented May 4, 2023

github-actions bot commented May 5, 2023

seabbs commented May 9, 2023

jamesmbaazam commented May 9, 2023

jamesmbaazam commented May 9, 2023

jamesmbaazam commented May 11, 2023

github-actions bot commented May 11, 2023

github-actions bot commented May 12, 2023

github-actions bot commented May 16, 2023

seabbs left a comment

seabbs May 16, 2023

jamesmbaazam May 16, 2023 •

edited

Loading

github-actions bot commented May 17, 2023

github-actions bot commented May 17, 2023

seabbs left a comment

seabbs left a comment

jamesmbaazam commented May 17, 2023 •

edited

Loading

seabbs commented May 17, 2023

github-actions bot commented May 17, 2023

Substitute nested ifelse with data.table::fcase() #383

Substitute nested ifelse with data.table::fcase() #383

Conversation

jamesmbaazam commented May 4, 2023 • edited Loading

github-actions bot commented May 4, 2023

github-actions bot commented May 5, 2023

seabbs commented May 9, 2023

jamesmbaazam commented May 9, 2023

jamesmbaazam commented May 9, 2023

jamesmbaazam commented May 11, 2023

github-actions bot commented May 11, 2023

github-actions bot commented May 12, 2023

github-actions bot commented May 16, 2023

seabbs left a comment

Choose a reason for hiding this comment

seabbs May 16, 2023

Choose a reason for hiding this comment

jamesmbaazam May 16, 2023 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented May 17, 2023

github-actions bot commented May 17, 2023

seabbs left a comment

Choose a reason for hiding this comment

seabbs left a comment

Choose a reason for hiding this comment

jamesmbaazam commented May 17, 2023 • edited Loading

seabbs commented May 17, 2023

github-actions bot commented May 17, 2023

Substitute nested `ifelse` with `data.table::fcase()` #383

Substitute nested `ifelse` with `data.table::fcase()` #383

jamesmbaazam commented May 4, 2023 •

edited

Loading

jamesmbaazam May 16, 2023 •

edited

Loading

jamesmbaazam commented May 17, 2023 •

edited

Loading