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

get_code to return character(1) instead of character(n) #173

Closed
gogonzo opened this issue Nov 27, 2023 · 2 comments
Closed

get_code to return character(1) instead of character(n) #173

gogonzo opened this issue Nov 27, 2023 · 2 comments
Assignees
@kartikeyakirar
Labels
core

Comments

@gogonzo
Copy link
Contributor

gogonzo commented Nov 27, 2023

  • get_code should return single string with all @code concatenated to a single string.

Please make relevant changes:

  • We don't need to use paste(get_code(...), collapse = "\n") everywhere anymore. Please locate all paste(collapse = "\n") in teal.modules.xyz and in teal where code is concatenated to a single string.
@gogonzo gogonzo added the core label Nov 27, 2023
@gogonzo gogonzo changed the title get_code to return character(1) instead of character(n) get_code to return character(1) instead of character(n) Nov 27, 2023
@kartikeyakirar kartikeyakirar self-assigned this Nov 27, 2023
@chlebowa
Copy link
Contributor

The internal function format_expression can also be removed. It is used in very few places and that number will soon drop further.

@kartikeyakirar kartikeyakirar mentioned this issue Nov 27, 2023
Merged
6 tasks
@vedhav
Copy link
Contributor

vedhav commented Nov 28, 2023

Maybe I'm missing something. Is there some reason that this issue is bucked into the wider refactor scope? I think it's fine to merge this directly into main without going through refactor.

kartikeyakirar added a commit that referenced this issue Nov 29, 2023
@kartikeyakirar @vedhav
get_code to return concatenated code string and removed `format_exp…
7ba7e80 
…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>
@m7pr m7pr mentioned this issue Dec 18, 2023
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kartikeyakirar kartikeyakirar

Labels
core
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@kartikeyakirar @gogonzo @vedhav @chlebowa