Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add definitions_id to Export to Google Buckets #9003

Closed
lilyclements opened this issue Jun 4, 2024 · 5 comments · Fixed by #9009
Closed

Add definitions_id to Export to Google Buckets #9003

lilyclements opened this issue Jun 4, 2024 · 5 comments · Fixed by #9009

Comments

@lilyclements
Copy link
Contributor

When this PR is merged, we get a new parameter into the "Export to Google Buckets" dialog.

This is very simple, and is just an additional label and ucrInput.

  • This can be placed below the "station ID:" label and input.
  • The label can say "Definitions ID:"
  • The ucrInput takes one alphanumeric value
  • The parameter is definitions_id
  • The corresponding function is export_r_instat_to_buckets (the base function, I think).
@jkmusyoka
Copy link
Contributor

jkmusyoka commented Jun 17, 2024

@lilyclements I am getting an "unused definition_id argument" when I try to complete the dialog as shown below
image

Is there anything wrong with the way I have chosen my def_ID?
Here is the full error message
image

@lilyclements
Copy link
Contributor Author

@jkmusyoka the error says “unused argument: definitions_id” this just means that you need to update your package. You’d want to update all three for lots of changes made too (unrelated to this, but just while you’re updating!) epicsawrap, epicsadata, and rpicsa

@jkmusyoka
Copy link
Contributor

@lilyclements. This is now resolved, thanks. I "forced" the updating.

But I now get a different error when I run the dialog again - "unused tmin and tmax arguments". See the error message below. Could it be a variable name issue?

image

@lilyclements
Copy link
Contributor Author

@jkmusyoka ah, the issue is that you have too recent of a version for the R-Instat version now! Damn. So, we don't want to have tmin/tmax in there anymore. You can do that by the R script for now, and I'll remove them in the dialog. Sorry about that.

I made a comment here about changes to make to the R-Instat dialog, meaning to write an issue but it slipped my mind. I'll do an issue now.

@jkmusyoka
Copy link
Contributor

Great

Can I also add to your list by suggesting that "start season probabilities" be renamed to "crop definitions" on the main and the subsequent sub dialog. This change will make it easier for Met staff to associate the "crop def" data frame to this sub dialog. The start season probabilities could easily be associated to start of rains which is not the case here.

I hope you agree?

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 a pull request may close this issue.

2 participants