Fixing the coord_polar bugs in graphics dialog #5541
Conversation
|
@maxwellfundi I suggest we make the parameters of the set up to the sub dialog compulsory, this way we cannot accidentally miss out options which cause a crash if not included. |
|
@dannyparsons I think that makes a lot of sense! It would not be possible to have this kind of event again in the future. |
|
@africanmathsinitiative/developers Someone to test this to make sure all the graphics dialog do not crash now when the plot options button is clicked. |
|
this looks good from my review |
|
Are you going to make those parameters compulsory as well? |
|
We can merge this now, I will do another PR with their changes.
Thank you
…On Mon, 7 Oct 2019, 16:34 Danny Parsons, ***@***.***> wrote:
Are you going to make those parameters compulsory as well?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5541>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACKYIIPGSF3EUHALIPTNMV3QNM3FJANCNFSM4I6BBCQQ>
.
|
|
You can include here, unless it might conflict with another pull request. That way we are sure this issue has been resolved in this pull request. |
|
@dannyparsons I actually have done the rest of the dialogs. It was useful to have this as the coordpolar parameter as mandatory. it actually helped to know where else the method is called and I fixed those dialogs too. |
instat/sdgPlots.vb
Outdated
| @@ -325,7 +325,7 @@ Public Class sdgPlots | |||
| ucrChkYaxisTickMarkLabelSize.AddParameterPresentCondition(False, "size", False) | |||
| End Sub | |||
|
|
|||
| Public Sub SetRCode(clsNewOperator As ROperator, Optional clsNewGlobalAesFunction As RFunction = Nothing, Optional clsNewYScalecontinuousFunction As RFunction = Nothing, Optional clsNewXScalecontinuousFunction As RFunction = Nothing, Optional clsNewLabsFunction As RFunction = Nothing, Optional clsNewXLabsTitleFunction As RFunction = Nothing, Optional clsNewYLabTitleFunction As RFunction = Nothing, Optional clsNewFacetFunction As RFunction = Nothing, Optional clsNewThemeFunction As RFunction = Nothing, Optional clsNewCoordPolarFunction As RFunction = Nothing, Optional clsNewCoordPolarStartOperator As ROperator = Nothing, Optional dctNewThemeFunctions As Dictionary(Of String, RFunction) = Nothing, Optional ucrNewBaseSelector As ucrSelector = Nothing, Optional strMainDialogGeomParameterNames() As String = Nothing, Optional bReset As Boolean = False) | |||
| Public Sub SetRCode(clsNewOperator As ROperator, clsNewCoordPolarFunction As RFunction, clsNewCoordPolarStartOperator As ROperator, Optional clsNewGlobalAesFunction As RFunction = Nothing, Optional clsNewYScalecontinuousFunction As RFunction = Nothing, Optional clsNewXScalecontinuousFunction As RFunction = Nothing, Optional clsNewLabsFunction As RFunction = Nothing, Optional clsNewXLabsTitleFunction As RFunction = Nothing, Optional clsNewYLabTitleFunction As RFunction = Nothing, Optional clsNewFacetFunction As RFunction = Nothing, Optional clsNewThemeFunction As RFunction = Nothing, Optional dctNewThemeFunctions As Dictionary(Of String, RFunction) = Nothing, Optional ucrNewBaseSelector As ucrSelector = Nothing, Optional strMainDialogGeomParameterNames() As String = Nothing, Optional bReset As Boolean = False) | |||
There was a problem hiding this comment.
Great, but please make everything up to and including dctNewThemeFunctions compulsory. All of these are needed and will cause problems otherwise.
I think the last few may be ok if not given so can stay optional.
|
Okay no problem.
…On Tue, 8 Oct 2019, 11:15 Danny Parsons, ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In instat/sdgPlots.vb
<#5541 (comment)>
:
> @@ -325,7 +325,7 @@ Public Class sdgPlots
ucrChkYaxisTickMarkLabelSize.AddParameterPresentCondition(False, "size", False)
End Sub
- Public Sub SetRCode(clsNewOperator As ROperator, Optional clsNewGlobalAesFunction As RFunction = Nothing, Optional clsNewYScalecontinuousFunction As RFunction = Nothing, Optional clsNewXScalecontinuousFunction As RFunction = Nothing, Optional clsNewLabsFunction As RFunction = Nothing, Optional clsNewXLabsTitleFunction As RFunction = Nothing, Optional clsNewYLabTitleFunction As RFunction = Nothing, Optional clsNewFacetFunction As RFunction = Nothing, Optional clsNewThemeFunction As RFunction = Nothing, Optional clsNewCoordPolarFunction As RFunction = Nothing, Optional clsNewCoordPolarStartOperator As ROperator = Nothing, Optional dctNewThemeFunctions As Dictionary(Of String, RFunction) = Nothing, Optional ucrNewBaseSelector As ucrSelector = Nothing, Optional strMainDialogGeomParameterNames() As String = Nothing, Optional bReset As Boolean = False)
+ Public Sub SetRCode(clsNewOperator As ROperator, clsNewCoordPolarFunction As RFunction, clsNewCoordPolarStartOperator As ROperator, Optional clsNewGlobalAesFunction As RFunction = Nothing, Optional clsNewYScalecontinuousFunction As RFunction = Nothing, Optional clsNewXScalecontinuousFunction As RFunction = Nothing, Optional clsNewLabsFunction As RFunction = Nothing, Optional clsNewXLabsTitleFunction As RFunction = Nothing, Optional clsNewYLabTitleFunction As RFunction = Nothing, Optional clsNewFacetFunction As RFunction = Nothing, Optional clsNewThemeFunction As RFunction = Nothing, Optional dctNewThemeFunctions As Dictionary(Of String, RFunction) = Nothing, Optional ucrNewBaseSelector As ucrSelector = Nothing, Optional strMainDialogGeomParameterNames() As String = Nothing, Optional bReset As Boolean = False)
Great, but please make everything up to and including dctNewThemeFunctions
compulsory. All of these are needed and will cause problems otherwise.
I think the last few may be ok if not given so can stay optional.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5541>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACKYIINB55O4VNDA4WTZS3LQNQ6TLANCNFSM4I6BBCQQ>
.
|
|
@dannyparsons this is now ready. All the necessary parameters have been made mandatory. |
|
could this be done to the |
|
@Ivanluv i did this for all the dialogs that uses sdgPlots. So that is taken care of too. Can someone test this quickly this morning, it needs to be merged because of the many file changes to avoid any conflicts. |
|
@maxwellfundi I tested this and all was fine from my end.Sorry I missed to see the changes to |
|
Okay great I will merge this then. Thank you. |
No description provided.