Some Changes Made in Length of Season Dialog

[MeSophie]

Fixes Partly #9117
@rdstern @lilyclements By waiting the code for End of Rains dialog, I made some changes in Length of season dialog. Please have a look.

[rdstern]

@MeSophie and @lilyclements this is excellent and nearly there already. And with a lovely bonus that you sort of forced me into!
As the end of the rains was not done yet, I compared 2 definitions of the start of the rains. Namely with and without a dry-spell condition. The bonus - and I suggest it is a pretty big bonus is that this is really useful itself. (I sometimes teach this, but I have to use the calculator, and this is much simpler - and now adds the status too. It is great!) And Sophie it is already working - that's very well done.

Just 1 simple improvement and one that may be a bit harder.
a) Could you make the length status variable into a factor.
b) There is also a question here for both of you, before making this change. When it is MORE the length is currently given as NA. I was hoping it would give the value, namely more than how many? However, I don't think it can, because the dialog doesn't know the end day.
Now I can see the end day. It is in the start of the rains dialog and is 366 in my example. Usually it will be the end day in the end of the rains dialog.
So I suggest we need another control. I suggest a checkbox below the 4 current receivers:

@MeSophie if you make the current controls a bit neater, then I think there will be space within the current size of the dialog.
a) Smaller space after the first and second receivers, to match the space after the 3rd receiver.
b) Move Length of season down a bit so the vertical space is the same as between the Status and the Comment.

The checkbox says End Day (Optional): If checked it opens an up-down with default 366, and can be any integer between 1 and 366.

When that's checked the length value gives the End Day - Start Day, whenever there is MORE in the status variable.

Now my suggestion, mainly for @lilyclements. I suggest we keep the length column as you have it now. And we add a further length column called the same as the length, but with _more added, e.g. diff_more above. This extra variable is not given if MORE is totally absent, i.e. there is no year where MORE is given..

I suggest that the diff_more variable, if given, is after the status variable. I show it in the example above. For that I just used the calculator, as follows:

diff_more <- ifelse(diff_status == "MORE",366-start_rain,diff )

This is exciting!

By the way I have not yet checked it works with multiple stations. I assume so, and that can come later.

[rdstern]

@MeSophie if you could make the length status into a factor, then I would approve now, with the other points coming in a new pull request.

[MeSophie]

@lilyclements Could you please help me with the best way to convert length_status to factor.
This is what I did but I'm afraid that tail function causes the same problem as previously.

start_end_status <- instat_calculation$new(type="calculation", function_exp="dplyr::case_when(is.na(start_rain_status) | is.na(end_rains_status) ~ NA_character_, start_rain_status == end_rains_status ~ as.character(start_rain_status), start_rain_status == FALSE & end_rains_status == TRUE ~ 'NONE', start_rain_status == TRUE & end_rains_status == FALSE ~ 'MORE')", result_name="length_status", calculated_from=list("dodoma_by_year"="start_rain_status","dodoma_by_year"="end_rains_status"), save=2)
length_rains_combined <- instat_calculation$new(type="combination", sub_calculation=list(length_of_season, start_end_status, start_end_status))
data_book$run_instat_calculation(calc=length_rains_combined, display=FALSE)
last_data_name <- tail(data_book$get_data_names(), n=1)
data_book$convert_column_to_type(data_name=last_data_name, col_names="length_status", to_type="factor")
rm(list=c("length_rains_combined", "length_of_season", "start_end_status"))

[lilyclements]

@MeSophie fortunately you've overcomplicated it for yourself! This time, we don't need to worry about calling "dodoma_by_year" in a fancy way because we use that data frame in the dialog code already
(the issue we had in Start of Rains was that we called dodoma, but generated and wanted to make changes to the linked data frame dodoma_by_year - but here, we are calling dodoma_by_year in the selector. Much nicer for us to handle).

So, instead of

last_data_name <- tail(data_book$get_data_names(), n=1)
data_book$convert_column_to_type(data_name=last_data_name, col_names="length_status", to_type="factor")

we can just do

data_book$convert_column_to_type(data_name=dodoma_by_year, col_names="length_status", to_type="factor")

[lilyclements]

@MeSophie I understand.

Instead of running

last_data_name <- tail(data_book$get_data_names(), n=1)

We should run

linked_data_name <- data_book$get_linked_to_data_name("dodoma", link_cols=c("year", "Station"))

This gives the name of the data frame which is linked to the dodoma data by year and Station.

However, this shouldn't be a problem in this instance because in the "Length of Season" dialog we have the "dodoma_by_year" (or the Moorings one) read in the selector.

[MeSophie]

@lilyclements Sorry I realised my mistake a bit late as you had already read my message. I really complicated the code because the DataName in the Length dialog can be taken directly from the selector.

[lilyclements]

@MeSophie all good! Glad you sorted it, but it's a good question nonetheless, and very observant for when this might occur in the Start/End of rains dialogs :)

[MeSophie]

@rdstern The conversion of the length_status into a factor is resolved. Please have a look.

[rdstern]

@MeSophie that's great. Just a trivial change for now:

Could you just make the label Length, so the fields for the variables are exactly below one another.
And could you move the Occurrence control up slightly, so the vertical spacing is the same.

[MeSophie]

@rdstern It is okay with the design now? I prefer to move the Length_status control a bit to the right to avoid a potential translation issue.

[rdstern]

Thanls @MeSophie
@N-thony if the code is ok it would be good to include this one too.