pecan
pecan copied to clipboard
Moved convert.input.R from utils to DB
To move convert.input
from utils
to DB
Copying the checklist we discussed in Slack here for easy reference:
- [x] check whether convert.input uses any unexported functions from PEcAn.utils. If so, see where else they're used and decide whether to move them to data.atm, copy them, or export them from utils
- [x] As one commit, move the convert.input function definition and man page from PEcAn.utils to PEcAn.DB.
- [x] As a separate commit, update the convert.input code to remove PEcAn.DB:: in front of calls to DB functions and to add PEcAn.utils:: in front of calls to utils. It's important to do it in a separate commit because otherwise Git will just show one giant blob deleted from utils and another added to DB, with no easy way to find the differences between them. Doing it in two steps makes it pretty easy to verify that the move part didn't change anything inside the blob
- [x] Review the function documentation and make any edits needed — I doubt it will, but do check
- [ ] Implement any needed deprecation stub in PEcAn.utils. The usual best practice is to deprecate functions slowly over several releases to give users time to update, but the circular dependency demands quicker action. Decision needed: After this change, should a call to PEcAn.utils::convert.input() fail with the function not found or should it do nothing but throw a message to use the version in PEcAn.DB?
- [x] Update the changelog and both packages’ NEWS files
- [x] Update all calls to convert.inputs elsewhere in PEcAn
- [x] Update Rcheck_reference.log, if needed, in all the packages touched by this PR
This looks great! Go head and tackle the last three points in my checklist above as part of this PR and we should be able to merge it all at once.
@infotroph To change the Rcheck_reference.log
PR #3000 is already opened. Should I merge it in this PR and close that one?
@nan1605 No, just update the files that this PR touches, probably by manual editing rather than a full new check run (i.e. only change the lines that need to change because of this PR). We'll see which PR gets merged first and resolve any conflicts after that in the remaining one.
@nanu1605 I think we'll need to add a line in both NEWS.md and changelog.md file stating that we've made a change in the function's name and why.
Replaced by #3026