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

Added Use as Numeric and Display as Factor checkboxes to Graphics dialog #8448

Merged
merged 7 commits into from
Sep 21, 2023

Conversation

MeSophie
Copy link
Contributor

Fixes #8428
@rdstern , @N-thony I added Use as Numeric and Display as Factor checkbox on Graphics dialog. Please check it.

@rdstern
Copy link
Collaborator

rdstern commented Sep 5, 2023

@MeSophie I think this change is easier than the code we started with. I need to check with @lilyclements, but suggest you keep the first checkbox and call it Group (or Group X - I am not sure. The second checkbox and code is deleted.
Then I show the code you had, and my change, which I prefer - found it on stack overflow of course!
Adding the group parameter does all we need!

# Dialog: Graphics

survey <- data_book$get_data_frame(data_name="survey")
##last_graph <- ggplot2::ggplot(data=survey, mapping=ggplot2::aes(x=as.numeric(variety), y=yield)) + ggplot2::geom_line() + theme_grey()
last_graph <- ggplot2::ggplot(data=survey, mapping=ggplot2::aes(x=variety,group=1, y=yield)) + ggplot2::geom_line() + theme_grey()
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"))

I have also made a new issue, on further possible improvements for this dialog, and suggest we now close this one, once this small change is merged.

@MeSophie
Copy link
Contributor Author

MeSophie commented Sep 5, 2023

I think this change is easier than the code we started with. I need to check with @lilyclements, but suggest you keep the first checkbox and call it Group (or Group X - I am not sure. The second checkbox and code is deleted.
Then I show the code you had, and my change, which I prefer - found it on stack overflow of course!
Adding the group parameter does all we need!

@rdstern I notice also that on your code you added geom_line. Will this geom_line function be added at the same time as the group parameter when the box is ticked?

@MeSophie
Copy link
Contributor Author

MeSophie commented Sep 6, 2023

@rdstern Please can you check the new change.

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.

@MeSophie this works, but the group=1 is added at the wrong time. If I do group before chooseing the geom, then it doesn't add group=1 into the code. If I untick, after getting the geom, and then tick again, it is fine. The action of ticking should add the code group=1 whatevwer the geom.

@MeSophie
Copy link
Contributor Author

MeSophie commented Sep 6, 2023

@rdstern I think it is okay now.

rdstern
rdstern previously approved these changes Sep 6, 2023
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.

@MeSophie this looks fine now. @lloyddewit there is more to improve on this dialog, but I would be happy to see this merged first. And the corresponding issue can then be closed.

Copy link
Contributor

@lloyddewit lloyddewit left a comment

Choose a reason for hiding this comment

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

@MeSophie looks good, just a couple of small suggestions

instat/dlgGeneralForGraphics.vb Outdated Show resolved Hide resolved
instat/dlgGeneralForGraphics.vb Outdated Show resolved Hide resolved
@lloyddewit
Copy link
Contributor

@rdstern if you can retest/approve, then we can merge, thanks

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.

@lloyddewit this still looks fine

@lloyddewit lloyddewit changed the title Added Use as Numeric and Display as Factor checkbox on Graphics Dialog Added Use as Numeric and Display as Factor checkboxes to Graphics dialog Sep 21, 2023
@lloyddewit lloyddewit merged commit 044fcf3 into IDEMSInternational:master Sep 21, 2023
2 checks passed
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.

Improve the ribbon geom and the general graph dialog?
3 participants