pecan PEcAn.utils package cleanup

Description

The utils package currently throws a LOT of messages at check time, contains a number of functions that appear to be vestigial or broken, and contains undeclared dependancies on other packages that in turn depend on utils. I propose to clean up as many of these as we can.

This is a big topic -- discuss, please. I'll link to sub-issues as we identify them.

Context

Possible Implementation

Delete?

[x] Everything in inst/get_mstmipvars.R -- this file looks like an old demo.
[x] model2netcdfdep and model2netcdf -- unused, have been marked as deprecated since 1.3.7!
[x] theme_border -- unused, relies on a hack that doesn't work in current versions of ggplot
[x] Are any datasets in base/utils/data/ unused? they add a LOT to the installed size of the package

Deprecate now, delete in a few releases?

[x] counter -- assigns to a global that nothing reads anymore #2086
[x] create.base.plot -- just wraps ggplot2::ggplot()
[ ] Most or all of status.R will be unneeded once #1660 is completed, but is essential until then.

Keep?

Apparently unused right now, but look potentially useful. Perhaps they should be more visibly documented.

[ ] as.sequence
[ ] distn.table.stats

Move to other packages?

(See also #1191)

[x] as.SafeList (function moved but need to change calls #1719)
[ ] date-time utils (to their own new package)
[x] plotting tools (to an existing package, maybe visualization?)
[ ] file-handling tools (maybe even just NetCDF-handling tools?)
[x] listToXml() to PEcAn.settings (function moved but need to change calls #1719)

Clean up existing code

Main goal: Reduce trivial output from devtools::check("base/utils") until we can tell it doesn't mask important messages

[x] Namespace janitorial work - Remove all library() calls - Move imported packages from Depends to Imports - :: for all out-of-package functions - Adress any remaining no visible binding for global... warnings (tasks above should fix most of them)
[x] Identify and resolve circular dependencies - Do any packages no longer depend on utils now that logger is separate? - do_conversions and convert.input both import a lot of other PEcAn packages. Can some of these move to Suggests instead of Depends?
[ ] Roxygen janitorial work
- Document all function parameters
- Check that \usage agrees with code
- Provide more examples

Sep 23 '17 00:09 infotroph

[x] Deprecate mstmipvar in favor of standard_vars? I could be convinced either way on this -- I know it's an eventual goal but there are still quite a few references to mstmipvar in other packages, especially PEcAn.ED2.

Sep 23 '17 15:09 infotroph

[ ] Fix install-time warning object ‘settings’ is created by more than one data call

Sep 23 '17 18:09 infotroph

I would also add:

[x] Move PEcAn.utils::fqdn() to PEcAn.remote (and change all calls to fqdn() elsewhere in PEcAn). It probably belongs there anyway, and then we can get rid of the current hack in is.localhost.

For reference:

Sep 27 '17 15:09 ashiklom

[x] #1722 Move do_conversions, get.results, run.write.configs etc to a new package possibly named PEcAn.workflow. Rationale: These are the functions that integrate calls from many other packages, so by design they depend on the rest of PEcAn. By including them here we guarantee circular dependencies with any other package that uses PEcAn.utils.

Sep 27 '17 15:09 infotroph

[x] Move Safelist to PEcAn.settings
[x] Move Safelist tests to match

Oct 05 '17 15:10 infotroph

[ ] What to do with base/utils/scripts/metutils.R?
[ ] Delete base/utils/scripts/time.constants.R?

Oct 06 '17 03:10 infotroph

If they are still used, https://github.com/PecanProject/pecan/blob/develop/base/utils/scripts/metutils.R belongs in models/ed/inst/scripts/ (/inst optional, depending on if access is needed when the package is loaded). These are ed specific met utilities.

Oct 06 '17 13:10 dlebauer

There's two things going on in metutils.R, the first is generation of ED met files, which has been completely replaced by met_process, and the second is the generation of met diagnostic plots, which we surprisingly don't do and seems like an idea worth migrating/reimplementing. So it should be moved to data.atmosphere or deleted.

This file is an example of something that you'll find a good bit of tucked away in PEcAn -- legacy code from how we were doing things before PEcAn. Basically, at one point I tried to dump the years and years of pre-existing scripts into the /inst folders of various packages so that we didn't have to solve every problem by starting from scratch (since a big point of PEcAn is that it's inefficient to solve every problem from scratch). Most of this legacy code has a strong ED bias, some of it is completely redundant at this point (historical met), but other parts deal with data types that we still haven't really tackled efficiently (forecast met, soils, topography) or only recently have tackled generalizing (cohort IC). Some, like metutils, include useful visualizations that could be generalized. Like utils (and my attic) it all needs a good going through and cleaning up, which should be a separate Issue, but it isn't pressing (except to avoid redundancies)

Oct 06 '17 14:10 mdietze

[x] Review datasets #1713

Oct 06 '17 15:10 infotroph

Necrobump! Related to recent Travis issues, a few more suggestions:

[ ] Move convert.inputs to PEcAn.workflow
[x] Move do.conversions to PEcAn.workflow #2168, #2169
[x] Move run.write.configs to PEcAn.workflow #2065
[x] Move everything from ensemble.R (#2031) and sensitivity.R into PEcAn.uncertainty

Oct 29 '18 17:10 ashiklom

[x] Move read.sa.output and write.sa.configs to uncertainty

Oct 31 '19 10:10 infotroph

This issue is stale because it has been open 365 days with no activity.

Oct 31 '20 00:10 github-actions[bot]