package clean-up

[pawelru]

• removed unnecessary empty first line in examples as this is actually being rendered
• removed osprey:: prefix in tests as this is redundant inside osprey
• fix roxygen warning about documenting package rd file
• fix ggplot lifecycle warnings about deprecated functions

Please review the changes carefully and let me know if there is something you don't like.

[github-actions]

Code Coverage Summary

Filename                   Stmts    Miss  Cover    Missing
-----------------------  -------  ------  -------  ---------
R/g_ae_sub.R                 344     344  0.00%    147-536
R/g_butterfly.R              183     183  0.00%    102-319
R/g_events_term_id.R         263     263  0.00%    129-490
R/g_heat_bygrade.R           210     210  0.00%    139-371
R/g_hy_law.R                  77      77  0.00%    119-199
R/g_patient_profile.R        390     390  0.00%    295-959
R/g_spiderplot_simple.R       20      20  0.00%    37-59
R/g_spiderplot.R             174     174  0.00%    109-345
R/g_swimlane.R               139     139  0.00%    119-320
R/g_waterfall.R              306     306  0.00%    146-506
R/stream_filter.R            126     126  0.00%    34-296
R/utils.R                    153     153  0.00%    43-362
TOTAL                       2385    2385  0.00%

Diff against main

Filename                 Stmts    Miss  Cover
---------------------  -------  ------  --------
R/g_heat_bygrade.R          +2      +2  +100.00%
R/g_patient_profile.R       +5      +5  +100.00%
R/g_spiderplot.R            -2      -2  +100.00%
TOTAL                       +5      +5  +100.00%

Results for commit: f41c2e0

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

[m7pr]

You know @pawelru why we have ADAE <- osprey::rADAE in examples instead of ADAE <- rADAE. It's becuase rADAE exists in a couple more other packages. So we are making sure you use the data directly from this package. For example {teal.transform} has this datasets https://github.com/insightsengineering/teal.transform/tree/main/data

[pawelru]

You know @pawelru why we have ADAE <- osprey::rADAE in examples instead of ADAE <- rADAE. It's becuase rADAE exists in a couple more other packages. So we are making sure you use the data directly from this package. For example {teal.transform} has this datasets https://github.com/insightsengineering/teal.transform/tree/main/data

True but those are not there - we are in osprey and not teal.osprey with all of the teal-family packages. At least R CMD CHECK does not complain ¯_(ツ)_/¯

[m7pr]

I know, imaginative situation is where you load osprey then you load teal.transform and then you run examples from osprey. Those datasets are exactly the same, but if they ever change, then you'd better have package:: to ensure which dataset you use.

Also if someone runs ?datasetname without the package:: prefix, (s)he may endup with a confusing list of multiple package containing one dataset. If you leave the prefix, the very highly probably someone will do the ? with a package prefix as well.

Not a big teal. We actually removed prefixes in examples for those datasets in teal.transform. Just trying to motivate why it has been there. I'm fine removing it.

[pawelru]

I know, imaginative situation is where you load osprey then you load teal.transform and then you run examples from osprey. Those datasets are exactly the same, but if they ever change, then you'd better have package:: to ensure which dataset you use.

This is actually quite likely - one could be a teal.osprey user - therefore with teal.transform as well as osprey. Alright, I will bring back the prefixes for this. Just because this namespace clash has been already identified. But the problem is more general: the same object name could be exported in multiple packages. I don't think there is a good and robust solution other than bring back prefixes to all (including exported !!) funs.

[donyunardi]

Just one small comment about but everything else looks.
I also tested all of the examples.

[donyunardi]

One thing I notice is that the function grob_part() is repeated twice in utils.R.

https://github.com/insightsengineering/osprey/blob/strict_tests/R/utils.R#L221-L235

https://github.com/insightsengineering/osprey/blob/strict_tests/R/utils.R#L327-L342

The contents are identical, but one has @keywords internal and one doesn't, which doesn't make sense.
We should remove one of them? We can create a new issue if we do.

[pawelru]

One thing I notice is that the function grob_part() is repeated twice in utils.R.

Great catch! I will just simply remove it here. This is meant to be a clean-up.

[m7pr]

This is actually quite likely - one could be a teal.osprey user - therefore with teal.transform as well as osprey. Alright, I will bring back the prefixes for this.

teal.osprey depends on teal.transform and osprey. I wonder which one gets attached first (teal.transformorosprey) when you attach teal.osprey`.