Skip to content

Conversation

@qdbell
Copy link
Contributor

@qdbell qdbell commented Nov 14, 2024

Description

Modified the usage of SticsOnR:set_param_xml() to utilise the vector functionality where many parameters can be passed to write them at once. This entailed removing the individual parameter writing calls and adding a conversion function pecan2stics() that takes pecan parameter names and converts them to the corresponding stics parameter names, along with any necessary unit conversions.

Motivation and Context

This change should speed up the use of PEcAn.STICS as there were many individual set_param_xml calls so it was slow to write configs.

Review Time Estimate

  • Immediately
  • Within one week
  • When possible

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • My name is in the list of CITATION.cff
  • I agree that PEcAn Project may distribute my contribution under any or all of
    • the same license as the existing code,
    • and/or the BSD 3-clause license.
  • I have updated the CHANGELOG.md.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@istfer
Copy link
Contributor

istfer commented Sep 23, 2025

sorry @infotroph I missed previous notifications on this, is there anything else remaining for us to check? thank you for keeping it updated

@infotroph
Copy link
Member

@istfer Looks like the current merge blocker is the package checks complaining about an unexported function:

  checking dependencies in R code ... WARNING
  '::' or ':::' import not declared from: ‘tibble’
  Unexported object imported by a ':::' call: ‘SticsRFiles:::gen_sol_xsl_file’
    See the note in ?`:::` about the use of this operator.

I only checked briefly, but it looks like you may have been using gen_sol_xsl_file as a workaround for a limitation in convert_xml2txt. If so, does using SticsRFiles >= 1.6.0 help? I see its NEWS includes "convert_xml2txt allows to convert sols.xml file to text files".

Alternately, if there's not a clean way to avoid using gen_sol_xsl_file then I suggest ignoring the message by pasting the check message lines I quoted above into Rcheck_reference.log replacing current lines 83-85.

Remove step of soil file conversion due to update in SticsRFiles.
@infotroph infotroph enabled auto-merge October 10, 2025 00:19
@infotroph infotroph added this pull request to the merge queue Oct 10, 2025
Merged via the queue into PecanProject:develop with commit ffecef7 Oct 10, 2025
19 of 26 checks passed
@infotroph infotroph mentioned this pull request Oct 16, 2025
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants