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

Line geoms #4455

Merged
merged 5 commits into from
Apr 6, 2018
Merged

Line geoms #4455

merged 5 commits into from
Apr 6, 2018

Conversation

bethanclarke
Copy link
Contributor

Changed some aes parameters to not mandatory in abline, hline and vline

clsgeom_abline.AddAesParameter("slope", strIncludedDataTypes:={"numeric"}, bIsMandatory:=True)
clsgeom_abline.AddAesParameter("intercept", strIncludedDataTypes:={"numeric"}, bIsMandatory:=True) '(required) intercept with the y axis of the line (the "b" in "y=ax+b")
clsgeom_abline.AddAesParameter("slope", strIncludedDataTypes:={"numeric"})
clsgeom_abline.AddAesParameter("intercept", strIncludedDataTypes:={"numeric"}) '(required) intercept with the y axis of the line (the "b" in "y=ax+b")
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are these not mandatory anymore? This seems to be mandatory from this reference -http://sape.inf.usi.ch/quick-reference/ggplot2/geom_abline

@bethanclarke
Copy link
Contributor Author

bethanclarke commented Mar 15, 2018

Hi Maxwell,

The slope and intercept parameters are mandatory but it is not mandatory to have these parameters set within the aes function.

We can have these parameters within aes function or within the ggplot or geom_abline function i.e. ``ggplot(data, aes(slope, intercept)) + geom_abline() - where we set variables to these parameters
or `ggplot(data, aes(), slope, intercept) + geom_abline()` - where we assign numeric values to these parameters

So if we have these as mandatory within in aes, then we cannot leave these blank on the dialog (layer dimensions tab) and then assign these parameters to a numeric value on the layer parameter tab

@dannyparsons
Copy link
Contributor

This is possibly another type of "mandatory" that we should have, where a parameter is mandatory somewhere in the geom but not in a particular place.

@maxwellfundi
Copy link
Contributor

@bethanclarke Is this ready for review yet?

@bethanclarke
Copy link
Contributor Author

bethanclarke commented Mar 27, 2018

Hi @maxwellfundi, yes it's ready to review. Thanks

@maxwellfundi
Copy link
Contributor

maxwellfundi commented Mar 27, 2018

Great, @africanmathsinitiative/developers someone can review this now.

@shadrackkibet
Copy link
Collaborator

Reviewing and testing this now.

@@ -1178,7 +1178,7 @@ Public Class ucrGeom

'adding layer parameters
'Geom_density layer parameters
clsgeom_violin.AddLayerParameter("draw_quantiles", "list", Chr(34) & "not(NULL)" & Chr(34), lstParameterStrings:={Chr(34) & "not(NULL)" & Chr(34), "0.25", "0.5", "0.75", "c(0.25, 0.5)", "c(0.25, 0.75)", "c(0.5,0.75)", "c(0.25,0.5,0.75)"}) 'If not(NULL) (default), draw horizontal lines at the given quantiles of the density estimate.
clsgeom_violin.AddLayerParameter("draw_quantiles", "list", Chr(34) & "NULL" & Chr(34), lstParameterStrings:={Chr(34) & "NULL" & Chr(34), "0.25", "0.5", "0.75", "c(0.25, 0.5)", "c(0.25, 0.75)", "c(0.5,0.75)", "c(0.25,0.5,0.75)"}) 'If not(NULL) (default), draw horizontal lines at the given quantiles of the density estimate - confusing instructions; it's say NULL is the default and when it not NULL and soemthing else then draw horizontal lines at the given quantiles of the density estimate
Copy link
Collaborator

Choose a reason for hiding this comment

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

From R documentation the default is not(NULL) why did you change this? . I didn't understand your comment here perhaps you could clarify.

Copy link
Collaborator

@shadrackkibet shadrackkibet Mar 28, 2018

Choose a reason for hiding this comment

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

ok, i see the confusion in R documentation. Should this parameter value have quotes around?

Copy link
Collaborator

@shadrackkibet shadrackkibet Mar 28, 2018

Choose a reason for hiding this comment

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

Perhaps i am doing something wrong here.I loaded survey dataset, on line plot dialog i have Fert as X Variable and Field as single variable then on plot options layer parameters i select geom_violin and check draw_quantiles checkbox i see the default is NULL then return back to the main dialog and click ok i found a bug.
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@shadrackkibet you are correct, NULL should not be in quotes in the code.

@maxwellfundi
Copy link
Contributor

@bethanclarke whats the status of this?

@bethanclarke
Copy link
Contributor Author

bethanclarke commented Apr 5, 2018 via email

@shadrackkibet
Copy link
Collaborator

shadrackkibet commented Apr 5, 2018

@bethanclarke could this be same issue i am experiencing ?
When i try to load data from Instat collection i get this "error", which i don't understand why.
image

@bethanclarke
Copy link
Contributor Author

Hi @shadrackkibet, I spoke to Danny and he said "You may have an older version of R which has this bug. Try updating to R 3.4.4 and see if that fixes it."

This fixed my problem however, I was having a different issue to you but it's worth updating R anyway to see if it fixes the issue you're having

@dannyparsons
Copy link
Contributor

@shadrackkibet which dataset did you try to load?

@shadrackkibet
Copy link
Collaborator

@shadrackkibet which dataset did you try to load?

Just immediately i click Browse Import from Library dialog loads and then i got above error.Additionally, when i click open from File in File menu and Open icon, i got the same issue.

@dannyparsons
Copy link
Contributor

It seems like the error is in displaying the File select dialog then. I'm not getting that on mine. Is anyone else getting this?

@maxwellfundi
Copy link
Contributor

Am not getting this either.

@shadrackkibet
Copy link
Collaborator

@dannyparsons this is now ready for you to review.

@shadrackkibet shadrackkibet mentioned this pull request Apr 6, 2018
@dannyparsons dannyparsons merged commit 9da1527 into IDEMSInternational:master Apr 6, 2018
@bethanclarke bethanclarke deleted the line-geoms branch June 7, 2018 10:47
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