Skip to content

Conversation

@rburghol
Copy link
Contributor

@COBrogan -- I just fixed a small bug, that is, the header info for odbc_delete was set to export odbc_post which seemed to not cause any trouble that I could discern, but I was chasing down why the WA metrics did not seem to be updating and I found something that could be related if there was indeed some real confusion as to the function in question.

Here's what I observed:

  • When running a segment of the James that showed NA for the WA_90_mgd in scenario 17, I openeed the slurm log. (nano slurm-404338.out )
  • The log showed that the routine called WA_export.R properly, and the routine seemingly executed correctly, printing out the valid WA results in the final line of the log as Calculating basinwide available flow as -1.55203619909502 + 0 / 90 = -1.552
  • BUT, there was no property to be found with the name WA_90_mgd on the runid_17 prop, though all others were there.l
  • I scrolled up in the slum log, lookoing for the output of the save command and noticed the message Final na/null check for pk vid val 9022116, so I checked the database for the pid 9022116, but found nothing.
  • Upon further investigation, the message Final na/null check for pk vid val 9022116 only occurs in the odbc_delete() function.
  • Now, there are probably reasons why the odbc_delete is called, but I could not find a reason inside the context of this WA script. It is the only script that I know of that has failed to save properties correctly.
  • So, I found this mislabeled export and thought maybe there was a connection.
  • I am doing this PR figuring it needs to happen one way or the other, and maybe it will help.

@rburghol rburghol requested a review from COBrogan as a code owner August 12, 2025 22:18
@rburghol rburghol merged commit c2f8bc0 into master Aug 12, 2025
1 check passed
@COBrogan
Copy link
Collaborator

COBrogan commented Aug 13, 2025

Good catch! It's weird this wasn't causing issues elsewhere. In the future, it seems we can use the generic @export rather than @export name to avoid similar issues. Roxygen should find the name of the function and insert it into the namespace easily enough
https://roxygen2.r-lib.org/articles/namespace.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants