insightsengineering / tlg-catalog

A catalog of Tables, Listings and Graphs (TLGs) created with NEST R packages
https://insightsengineering.github.io/tlg-catalog/
Other
20 stars 8 forks source link

Sanitize tables #102

Closed edelarua closed 1 year ago

edelarua commented 1 year ago

Part of https://github.com/insightsengineering/tern/issues/992

github-actions[bot] commented 1 year ago

Unit Tests Summary

    1 files  2 suites   1m 1s :stopwatch:   26 tests 0 :heavy_check_mark:   26 :zzz: 0 :x: 275 runs  0 :heavy_check_mark: 275 :zzz: 0 :x:

Results for commit 96705248.

edelarua commented 1 year ago

@Melkiades none of the snapshots were changed.

I went through all the tables in TLG-C and checked with assert_valid_table() - aside from these tables, there are a few that use tern functions that need to be refactored to be sanitized. I have updated the issue in tern with a more detailed list of what still needs to be done to sanitize all tables: https://github.com/insightsengineering/tern/issues/992#issue-1777334964

pawelru commented 1 year ago

@pawelru do you think we should keep a NEWS.md updated also for the tlg-catalog?

Only if this is used and auto-checked. The latter is not true and I have some doubts for the former. You have to know that this is not a new idea and we used to have it some time ago. But usually developers forgot to update it and it was a dead repo artifact hence we removed it.

Melkiades commented 1 year ago

@pawelru do you think we should keep a NEWS.md updated also for the tlg-catalog?

Only if this is used and auto-checked. The latter is not true and I have some doubts for the former. You have to know that this is not a new idea and we used to have it some time ago. But usually developers forgot to update it and it was a dead repo artifact hence we removed it.

I think we have it in scda.test but it is not used a lot. Maybe, it is better to let it go for these two repos