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

Improvements to Describe > Graph > Scatterplot #9001

Merged
merged 12 commits into from
Jun 19, 2024

Conversation

derekagorhom
Copy link
Contributor

Fixes Partly #8800
Replaces #8927
This PR fixes parts e, f, and g. This PR is ready for review.
@fran2or, @rdstern.

@rdstern
Copy link
Collaborator

rdstern commented Jun 4, 2024

@derekagorhom thanks for working on this. That's progress. Here is the dialog now:

image

a) Could the group box for the point, count and jitter be a bit narrower. Ot doesn't need to go right to the edge of the dialog.
b) Very small, but the control saying Identity, seems a bit lower. Could you check the laignment.
c) The Colour control is not yet giving the colours. Please change it to the Colour options.
d) I haven;t checked the coding change I requested in point g) Could you please confirm that you made the change.

Then I'm looking forward to the rest. I'm excited by being able to add the Side plots.

@derekagorhom
Copy link
Contributor Author

@rdstern I have made the code changes
for the item g) when the user checks either the shape or size or both there will appear in the geom_point() otherwise they will not appear there.
see below when the checkboxes are ticked
survey <- data_book$get_data_frame(data_name="survey") last_graph <- ggplot2::ggplot(data=survey, mapping=ggplot2::aes(colour=variety, y=yield, x=fert)) + ggplot2::geom_point(size=2.2, shape="square") + theme_grey() + ggplot2::facet_wrap(facets= ~ village) data_book$add_object(data_name="survey", object_name="last_graph", object_type_label="graph", object_format="image", object=check_graph(graph_object=last_graph)) data_book$get_object_data(data_name="survey", object_name="last_graph", as_file=TRUE) rm(list=c("last_graph", "survey"))

and when the checkboxes are not ticked
survey <- data_book$get_data_frame(data_name="survey") last_graph <- ggplot2::ggplot(data=survey, mapping=ggplot2::aes(colour=variety, x=fert, y=yield)) + ggplot2::geom_point() + theme_grey() + ggplot2::facet_wrap(facets= ~ village) data_book$add_object(data_name="survey", object_name="last_graph", object_type_label="graph", object_format="image", object=check_graph(graph_object=last_graph)) data_book$get_object_data(data_name="survey", object_name="last_graph", as_file=TRUE) rm(list=c("last_graph", "survey"))

@rdstern
Copy link
Collaborator

rdstern commented Jun 5, 2024

@derekagorhom here is a snapshot:

image

Could you change Colour Option to just Colour. And can you see that currently these are not colours. They should offer the same colour options as in the sub-dialog for the Count.

@derekagorhom
Copy link
Contributor Author

@rdstern I have made the change can you review this again
Thanks

@rdstern
Copy link
Collaborator

rdstern commented Jun 7, 2024

@derekagorhom looks good. Is there a reason for omitting the Pick Colour option from the list on the dialog? It is on the sub-dialog.

@derekagorhom
Copy link
Contributor Author

@rdstern apologies
I thought i had added that option along with the other colours

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.

@derekagorhom I am now approving. There remains the ggside to include, but that is a pretty big job. @N-thony over to you to check and merge.
I am still concenred that the dialog doesn't look very professional. The spacing and lining of the controls still look as though there could be improvements, but I am poor on these design points. Antoine you are better, but I also think there are more important tasks for now.

@N-thony N-thony merged commit 8472a35 into IDEMSInternational:master Jun 19, 2024
2 checks passed
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.

4 participants