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

Improved the Climatic >Prepare>Length of Season dialog #9427

Closed

Conversation

MeSophie
Copy link
Contributor

@MeSophie MeSophie commented Feb 7, 2025

Fixes #9402
@rdstern @N-thony I replaced the Up-Down control with the Day Range sub_dialog in the Length of season dialog. Please have a look.

@rdstern
Copy link
Collaborator

rdstern commented Feb 7, 2025

@MeSophie that's absolutely the right idea. But:
a) Can you make the receiving control narrower - it just has the day number so can be much smaller.
b) I'm happy that you adapted the control like that. But there are 2 problems:

  1. The data uses the shifted year, while you are not using that here. So 30 April should be about day 305 starting from 1 July.
  2. You have messed up the settings too generally, see below:

image

I went back to the start of the rains, and now can't do the start day here either. The length dialog should not affect the control for any other dialog.

@MeSophie MeSophie marked this pull request as draft February 12, 2025 10:25
@MeSophie MeSophie marked this pull request as ready for review February 12, 2025 12:48
@MeSophie
Copy link
Contributor Author

@rdstern Please can you test the dialog again?

image
If this is related with Length of season dialog then I'm not sure that the range day can recognize the shifted year since we don't have any DOY receivers here.

@rdstern
Copy link
Collaborator

rdstern commented Feb 12, 2025

@MeSophie I am very sorry, as you have now stopped the interaction between the controls on the dialogs. But I have now thought of a much simpler way of doing this! And it is sufficiently different that - if you and @lilyclements agree, then I suggest you close this pull request and start again on the length dialog.
It is working well and here is an example for Zambia:

image

So it is doing fine, except, with your current control I had to pretend the season was finishing on 31 October. (It actually was 30 April which is day 305 in the shifted year - which you don't give!)

Notice 305 is the day I want and it is there in the End_season_filled variable! Hence my new suggestions:

image

a) Add another receiver, with label End_Filled
b) Complete these controls with defaults where you can from the names. For the start it should be easy. For the end use end season as the defaults, and end rains otherwise.
c) For the End_Filled, that is only available for the end of the season. Fill that control if you are able.
d) The Length More checkboa is disabled unless End_Filled is not empty.
e) The Length_More checkbox becomes like the 2 controls above. So with length_more as the default name.
f) There is no day_range control at all. Instead use max(End_Filled) as the day number you need. This will either be correct, which corresponds to FALSE in the end Season, or it won't matter, because it won't be used, i.e. there are no FALSE rows in the End and hence no More rows in the length!

Very sorry for the extra work, but I like this solution much more!

@rdstern
Copy link
Collaborator

rdstern commented Feb 12, 2025

@MeSophie you didn't read my suggestions above enough. I was amazed you changed the dialog sao quickly and no saw you didn't. I strongly suggest you close this pull request and then you should find the new pull request is much easier to implement. Happy to chat over skype if you wish.

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.

Improve The Climatic > Prepare>Length of Season Dialog
2 participants