insightsengineering / nestdevs-tasks

0 stars 0 forks source link

Remove `chunk` code base from teal #23

Closed donyunardi closed 1 year ago

donyunardi commented 1 year ago

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

Tasks

donyunardi commented 1 year ago

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

gogonzo commented 1 year ago

@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 commented 1 year ago

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

donyunardi commented 1 year ago

@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 commented 1 year ago

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

donyunardi commented 1 year ago

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: https://github.com/insightsengineering/teal.modules.hermes/issues/307