Fix reproducibility message

[nikolas-burkoff]

Along with PRs in slice insightsengineering/teal.slice#18 and data insightsengineering/teal.data#20 this fixes insightsengineering/teal.data#4

I could do with adding some tests here but I'm a little unsure of the best way to do it

Test with e.g. insightsengineering/teal.data#4

---

@gogonzo @pawelru

I really dislike the design we have here - we are tightly coupling TealData with FilteredData (even if we later allow FilteredData to be created with a list they are still very tightly coupled) - it feels like FilteredData is doing too much - it is both storing the data and its arrtibutes (metadata, check, keys) which duplicated TealData and also handles the filtering.

In my opinion they should be completely separate - the data/metadata should live in TealData and be available to module developers - the filterpanel should then not contain copies of the data and should be like the data_extract/data_merge which can use the data and output calls/data to also be used by module developers.

This then properly decouples the 2 objects - because at the moment if we want to introduce another filterpanel (maybe a more complex one with hierarchical filters etc.) then we need to duplicate all of the copying of TealData-like objects into that filterpanel - and also if we want to change the underlying TealData object (i.e. adding metadata etc.) then we have to pass that through to the FilteredDatasets class

[github-actions]

Unit Tests Summary

    1 files    12 suites   50s ⏱️
  79 tests   79 ✔️ 0 💤 0 ❌
181 runs  181 ✔️ 0 💤 0 ❌

Results for commit 829f00c.

♻️ This comment has been updated with latest results.

[kpagacz]

I don't really think this reproducibility message should be believed at all. Any module developer has access to the FilteredData object passed to a module which in turn means they can do whatever they want with it, including changes to the code stored in any data classes we store anywhere. Since any teal application using foreign modules passes the object to it, it can be tampered with and any results taken from the said object shouldn't be trusted.

Therefore slapping our (teal developers) certificate of being correct on something that can be incorrect seems like a bad idea. Generally speaking, R does have very poor security regarding objects in its workspace. Basically, everything is mutable. To achieve any level of security, we would have to introduce an external tool.

[nikolas-burkoff]

@kpagacz
Also the reproducibility check does not consider connectors as they are assumed to be correct, but of course re-running code which pulls data from an external system cannot guarantee reproducibility.

It's also much more useful to be able to look at the reproducibility of the entire SRC rather than just the filter - as a module developer you don't get any help making sure that your code in teal is correct - only the filterpanel

[gogonzo]

I think this deserves NEWS entry under BUG fixes

[gogonzo]

👍 please fix the NEWS