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

Remove chunk code base from teal #23

Closed
donyunardi opened this issue Apr 6, 2023 · 6 comments
Closed

Remove chunk code base from teal #23

donyunardi opened this issue Apr 6, 2023 · 6 comments
Labels

Comments

@donyunardi
Copy link
Collaborator

donyunardi commented Apr 6, 2023

Summary

This feature is deprecated in the last release, in favor of qenv.

We have made significant efforts to replace the usage of chunk with qenv in teal modules. However, to ensure a safe transition, we need to release the teal modules with the qenv implementation before proceeding to remove the chunk code base entirely.

Definition of Done

  • Remove chunk code base from teal framework and modules

Tasks

@donyunardi donyunardi added core good first issue Good for newcomers labels Apr 6, 2023
@donyunardi donyunardi changed the title Remove chunk code base Remove chunk code base from teal Apr 14, 2023
@donyunardi donyunardi transferred this issue from insightsengineering/teal.code Apr 14, 2023
@donyunardi donyunardi removed the good first issue Good for newcomers label Apr 14, 2023
@donyunardi
Copy link
Collaborator Author

@gogonzo
We can't do this until all teal modules are released, right?

@gogonzo
Copy link

gogonzo commented May 26, 2023

@donyunardi we've removed chunks from all modules and they have been released already without chunks. There are just few occurrences of chunks in other packages and they should be removed also (as these elements containing chunks are not used also). I wonder what chunks does in teal.modules.hermes - @danielinteractive ?

Occurrences of the chunks in the codebase:
https://github.com/search?q=org%3Ainsightsengineering%20chunks&type=code

@danielinteractive
Copy link

@gogonzo Fortunately we don't use chunks at all in teal.modules.hermes or teal.modules.helios. So no concerns from our side.

@donyunardi
Copy link
Collaborator Author

donyunardi commented May 26, 2023

@gogonzo

we've removed chunks from all modules and they have been released already without chunks

I believe you are referring to the update we implemented to replace the usage of chunks with qenv in teal modules.
However, we haven't release another stable version of the modules after the implementation.

Here's an example for tmg where we still have chunks occurrences in latest stable release v0.2.15
https://github.com/insightsengineering/teal.modules.general/blob/v0.2.15/R/tm_a_pca.R#L266

If we remove chunks before introducing a new stable release of tmg, I am worried that some tests will fail.

Therefore, before we can completely eliminate the chunks code base, we will need to release another stable version of tmg, and other teal modules that was affected with qenv implementation. Once this new release is available, we can safely proceed with the removal of the chunks code.

Please let me know your thought on this.

@danielinteractive
As @gogonzo pointed out, I also see chunks occurrences in teal.modules.hermes latest main.
Click here to see the two related files.

@danielinteractive
Copy link

Those are Just design files, which are not part of the R package.

@donyunardi
Copy link
Collaborator Author

donyunardi commented May 30, 2023

Thank you, @danielinteractive. I will leave it to you to update that section, but it should not hinder/block the progress of this roadmap. I will close the tmh issue associated with this roadmap:
insightsengineering/teal.modules.hermes#307

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants