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

refactor branch for teal.code #174

Merged
merged 8 commits into from
Dec 8, 2023
Merged

refactor branch for teal.code #174

merged 8 commits into from
Dec 8, 2023

Conversation

kartikeyakirar
Copy link
Contributor

This is a feature branch, DON'T MERGE and DON'T COMMENT. PR open to have a constant checks. We will start review this PR as a whole in the sprint 4.

Copy link
Contributor

github-actions bot commented Nov 27, 2023

badge

Code Coverage Summary

Filename                 Stmts    Miss  Cover    Missing
---------------------  -------  ------  -------  ---------
R/qenv-concat.R             10       0  100.00%
R/qenv-constructor.R        16      13  18.75%   54-93
R/qenv-eval_code.R          52       2  96.15%   100, 109
R/qenv-get_code.R           20       0  100.00%
R/qenv-get_var.R            19       0  100.00%
R/qenv-get_warnings.R       24       0  100.00%
R/qenv-join.R               46       0  100.00%
R/qenv-replace_code.R       10       0  100.00%
R/qenv-show.R                1       1  0.00%    19
R/qenv-within.R              8       0  100.00%
R/utils.R                   10       0  100.00%
TOTAL                      216      16  92.59%

Diff against main

Filename                 Stmts    Miss  Cover
---------------------  -------  ------  --------
R/qenv-get_code.R           +2       0  +100.00%
R/qenv-replace_code.R       +2       0  +100.00%
R/utils.R                   -2       0  +100.00%
TOTAL                       +2       0  +0.07%

Results for commit: c82f1ad

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

Copy link
Contributor

github-actions bot commented Nov 27, 2023

Unit Tests Summary

    1 files    10 suites   0s ⏱️
  68 tests   68 ✔️ 0 💤 0
139 runs  139 ✔️ 0 💤 0

Results for commit c82f1ad.

♻️ This comment has been updated with latest results.

gogonzo and others added 2 commits November 28, 2023 11:56
…ression` (#176)

this fixes #173


In this pull request, I've made updates to both the `get_code` and
`replace_code` methods. Now, when the `deparse `arg is set to TRUE,
these methods will return a concatenated string. Specifically, the
`get_code` method will return character(0) when used with `new_qenv(),`
provided no code has been set. As for `replace_code,` it will first
split the input string using "\n", then replace the last string in this
sequence. Finally, it concatenates these strings again before replacing
the code.

Removed `format_expression` function.

Example
```

q <- new_qenv()
get_code(q) # return character(0)
replace_code(q, "i <- iris")  # @code return character(0)

qq <- within(q, i <- iris)
qq <- within(qq, m <- mtcars)
get_code(qq) # return "i <- iris\nm <- mtcars"

replacement <- "i <- mtcars"
qr <- replace_code(qq, replacement)
get_code(qr) #return "i <- iris\ni <- mtcars"
```

Also review following modules for PR for changes.

- [ ] teal insightsengineering/teal#976
- [ ] teal.data
insightsengineering/teal.data#206
- [ ] teal.modules.general
insightsengineering/teal.modules.general#615
- [ ] teal.modules.clinical
insightsengineering/teal.modules.clinical#898
- [ ] teal.goshawk
insightsengineering/teal.goshawk#250
- [ ] teal.osprey
insightsengineering/teal.osprey#244

---------

Signed-off-by: kartikeya kirar <kirar.kartikeya1@gmail.com>
Co-authored-by: Vedha Viyash <49812166+vedhav@users.noreply.github.com>
Copy link
Contributor

github-actions bot commented Nov 29, 2023

Unit Test Performance Difference

Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
utils 💀 $0.01$ $-0.01$ format_expression_turns_expression_calls_or_lists_thereof_into_character_strings_without_curly_brackets

Results for commit 66cfc68

♻️ This comment has been updated with latest results.

@gogonzo gogonzo enabled auto-merge (squash) December 8, 2023 11:00
Copy link
Contributor

@gogonzo gogonzo left a comment

Choose a reason for hiding this comment

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

👍

@gogonzo gogonzo merged commit a017cec into main Dec 8, 2023
25 checks passed
@gogonzo gogonzo deleted the refactor branch December 8, 2023 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants