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

use the formula environment as the newly created quosure environment. #5886

Merged
merged 6 commits into from May 19, 2021

Conversation

romainfrancois
Copy link
Member

closes #5881

# pak::pak("msberends/AMR@v1.6.0")
library(AMR)
library(skimr)

skim(example_isolates)
Name example_isolates
Number of rows 2000
Number of columns 49
_______________________
Column type frequency:
character 2
Date 1
factor 1
logical 3
mo 1
numeric 1
rsi 40
________________________
Group variables None

Data summary

Variable type: character

skim_variable n_missing complete_rate min max empty n_unique whitespace
gender 0 1 1 1 0 2 0
patient_id 0 1 6 6 0 981 0

Variable type: Date

skim_variable n_missing complete_rate min max median n_unique
date 0 1 2002-01-02 2017-12-28 2009-07-31 1140

Variable type: factor

skim_variable n_missing complete_rate ordered n_unique top_counts
hospital_id 0 1 FALSE 4 D: 762, B: 663, A: 321, C: 254

Variable type: logical

skim_variable n_missing complete_rate mean count
ward_icu 0 1 0.32 FAL: 1354, TRU: 646
ward_clinical 0 1 0.73 TRU: 1464, FAL: 536
ward_outpatient 0 1 0.06 FAL: 1880, TRU: 120

Variable type: mo

skim_variable n_missing complete_rate unique_total gram_negative gram_positive top_genus top_species
mo 0 1 90 NA NA Staphylococcus Escherichia coli

Variable type: numeric

skim_variable n_missing complete_rate mean sd p0 p25 p50 p75 p100 hist
age 0 1 71.06 14.12 14 63 74 82 97 ▁▁▃▇▅

Variable type: rsi

skim_variable n_missing complete_rate ab_name count_R count_S count_I prop_R prop_S prop_I
PEN 371 0.81 NA 1201 428 11 0.74 0.26 0.01
OXA 1635 0.18 NA 114 251 0 0.31 0.69 0.00
FLC 1057 0.47 NA 278 665 0 0.29 0.71 0.00
AMX 650 0.68 NA 804 546 3 0.60 0.40 0.00
AMC 121 0.94 NA 446 1433 91 0.24 0.76 0.05
AMP 650 0.68 NA 804 546 3 0.60 0.40 0.00
TZP 999 0.50 NA 126 875 13 0.13 0.87 0.01
CZO 1554 0.22 NA 199 247 2 0.45 0.55 0.00
FEP 1276 0.36 NA 103 621 1 0.14 0.86 0.00
CXM 211 0.89 NA 470 1319 22 0.26 0.74 0.01
FOX 1182 0.41 NA 224 594 8 0.27 0.73 0.01
CTX 1057 0.47 NA 146 797 1 0.15 0.85 0.00
CAZ 189 0.91 NA 1204 607 0 0.66 0.34 0.00
CRO 1057 0.47 NA 146 797 1 0.15 0.85 0.00
GEN 145 0.93 NA 456 1399 27 0.25 0.75 0.01
TOB 649 0.68 NA 465 886 7 0.34 0.66 0.01
AMK 1308 0.35 NA 441 251 0 0.64 0.36 0.00
KAN 1529 0.24 NA 471 0 0 1.00 0.00 0.00
TMP 501 0.75 NA 571 928 10 0.38 0.62 0.01
SXT 241 0.88 NA 361 1398 6 0.21 0.79 0.00
NIT 1257 0.37 NA 127 616 51 0.17 0.83 0.07
FOS 1649 0.18 NA 148 203 0 0.42 0.58 0.00
LNZ 977 0.51 NA 709 314 0 0.69 0.31 0.00
CIP 591 0.70 NA 228 1181 69 0.16 0.84 0.05
MFX 1789 0.11 NA 71 140 4 0.34 0.66 0.02
VAN 139 0.93 NA 712 1149 0 0.38 0.62 0.00
TEC 1024 0.49 NA 739 237 0 0.76 0.24 0.00
TCY 800 0.60 NA 357 843 23 0.30 0.70 0.02
TGC 1202 0.40 NA 101 697 0 0.13 0.87 0.00
DOX 864 0.57 NA 315 821 7 0.28 0.72 0.01
ERY 106 0.95 NA 1084 810 9 0.57 0.43 0.00
CLI 480 0.76 NA 930 590 4 0.61 0.39 0.00
AZM 106 0.95 NA 1084 810 9 0.57 0.43 0.00
IPM 1111 0.44 NA 55 834 10 0.06 0.94 0.01
MEM 1171 0.41 NA 49 780 0 0.06 0.94 0.00
MTR 1966 0.02 NA 5 29 0 0.15 0.85 0.00
CHL 1846 0.08 NA 33 121 0 0.21 0.79 0.00
COL 360 0.82 NA 1331 309 0 0.81 0.19 0.00
MUP 1730 0.14 NA 16 254 3 0.06 0.94 0.01
RIF 997 0.50 NA 698 305 2 0.70 0.30 0.00

Created on 2021-05-17 by the reprex package (v2.0.0)

@romainfrancois romainfrancois added this to the 1.0.7 milestone May 17, 2021
Copy link
Member

@lionel- lionel- left a comment

Choose a reason for hiding this comment

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

I was planning for R 4.1 lambdas and trying to have formulas and native lambdas use the same code path. However I think that's a mistake, we should have different code paths.

  • Evaluated formulas can be inlined.
  • Evaluated functions cannot be inlined.

So there will be performance implications for using list(~ f(.), ~ g(.)) over list(\(.) f(.), \. g(.)). The latter won't be inlined. Maybe we will be able to do better with strictness annotations.

The bottom line is that I think your change makes sense and we should accept the different semantics for r-lib and native lambdas. I have added a unit test to cover the local lookup case.

@romainfrancois romainfrancois merged commit c5d3df3 into master May 19, 2021
@romainfrancois romainfrancois deleted the as_across_fn_call_formul_env branch May 19, 2021 10:40
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.

Changes in how internal functions are handled in across() breaks AMR
2 participants