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

Fixed bugs in PICSA Rainfall Dialogue #7549

Conversation

derekagorhom
Copy link
Contributor

@derekagorhom derekagorhom commented Jun 9, 2022

Fixes (partially) #7534
This PR fixes the bugs recently found on the PICSA rainfall graph dialogue.
@N-thony @Vitalis95 can you test.

There seems to be an issue with the dialogue.

  1. In the Single variable; when using either Line or Point Options the user has to automatically select the variable it wants to use as the Y variable. (I tried correcting that but it created more problems that solutions).
    to show proof:
    When using the dialogue without Line/Point options:
    ghana <- data_book$get_data_frame(data_name="ghana")
    last_graph <- ggplot2::ggplot(data=ghana,mapping=ggplot2::aes(y=as.numeric(x=rainfall), x=month_abbr))+....

When using the Dialogue with Line/Point options:
ghana <- data_book$get_data_frame(data_name="ghana")
last_graph <- ggplot2::ggplot(data=ghana, mapping=ggplot2::aes(y=rainfall, x=month_abbr)) +...

And finally when using the dialogue with the multiple variable:
ghana <- data_book$get_data_frame(data_name="ghana", stack_data=TRUE, measure.vars=c("min_temperature","max_temperature"))
last_graph <- ggplot2::ggplot(data=ghana, mapping=ggplot2::aes(y=as.numeric(x=value), colour=variable, x=month_abbr)) +....

This PR does not fix issue #7534...but rather the bugs reported by Antoine and Vitalis

@rdstern
Copy link
Collaborator

rdstern commented Jun 13, 2022

Please can @Vitalis95 and @N-thony check and report on this fix. I can't find the corresponding issues from them, so I'll wait for their evaluation.

@N-thony
Copy link
Collaborator

N-thony commented Jun 13, 2022

This PR fixes the bugs recently found on the PICSA rainfall graph dialogue. @N-thony @Vitalis95 can you test.

There seems to be an issue with the dialogue.

  1. In the Single variable; when using either Line or Point Options the user has to automatically select the variable it wants to use as the Y variable. (I tried correcting that but it created more problems that solutions).
    to show proof:
    When using the dialogue without Line/Point options:
    ghana <- data_book$get_data_frame(data_name="ghana")
    last_graph <- ggplot2::ggplot(data=ghana,mapping=ggplot2::aes(y=as.numeric(x=rainfall), x=month_abbr))+....

When using the Dialogue with Line/Point options: ghana <- data_book$get_data_frame(data_name="ghana") last_graph <- ggplot2::ggplot(data=ghana, mapping=ggplot2::aes(y=rainfall, x=month_abbr)) +...

And finally when using the dialogue with the multiple variable: ghana <- data_book$get_data_frame(data_name="ghana", stack_data=TRUE, measure.vars=c("min_temperature","max_temperature")) last_graph <- ggplot2::ggplot(data=ghana, mapping=ggplot2::aes(y=as.numeric(x=value), colour=variable, x=month_abbr)) +....

This PR does not fix issue #7534...but rather the bugs reported by Antoine and Vitalis

@derekagorhom I thought this will be done in the same PR i.e #7534 since it concerns the dialogue you are working on there.

@derekagorhom
Copy link
Contributor Author

Yes but since @Vitalis95 was working on the same dialogue, i wanted to be done this bug and continue with #7534

@Vitalis95
Copy link
Contributor

@rdstern , the bugs I found after pulling the changes made on PR #7505 and those bugs I raised them in the issue #7534 and are now solved.
those bugs were -
i) the argument 'x' was missing
ii) when you go back to the dialog, the variable in the single receiver you had added was not there again and when you add it back , the test OK now was disabled
Also the bug @N-thony raised where, When you click on multiple receive, the single receiver is still present is also sorted

@N-thony
Copy link
Collaborator

N-thony commented Jun 14, 2022

@rdstern , the bugs I found after pulling the changes made on PR #7505 and those bugs I raised them in the issue #7534 and are now solved. those bugs were - i) the argument 'x' was missing ii) when you go back to the dialog, the variable in the single receiver you had added was not there again and when you add it back , the test OK now was disabled Also the bug @N-thony raised where, When you click on multiple receive, the single receiver is still present is also sorted

@rdstern could you also test?

Copy link
Collaborator

@rdstern rdstern left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to work fine, except for the bug I found, which I gather will be fixed separately. So happy to approve. It will be good, so that @Vitalis95 can work on other aspects of this dialogue.

@lloyddewit lloyddewit changed the title Bug fixes on PICSA Rainfall Dialogue Fixed bugs in PICSA Rainfall Dialogue Jun 15, 2022
@lloyddewit lloyddewit added the bug label Jun 15, 2022
@lloyddewit lloyddewit merged commit 9be2dda into IDEMSInternational:master Jun 15, 2022
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.

None yet

5 participants