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 reorder control to histogram plot #8547

Merged
merged 4 commits into from
Sep 22, 2023

Conversation

Vitalis95
Copy link
Contributor

Fixes #8335
@Fidel365 , take a look and test it before @rdstern and @N-thony

@Fidel365
Copy link
Contributor

@rdstern and @N-thony, this is ready for your review. There is a small change for the density ridges checkbox that only displays the checkbox if there is a factor to edit. You can weigh on that and if the change is needed, then you can also merge PR #8548 alongside this.

@rdstern
Copy link
Collaborator

rdstern commented Sep 21, 2023

@Fidel365 I think you have tested by now, have you? If so then did you notice that the initial problem I had, still remains?
@Vitalis95 if I use histogram and add the factor and then reverse all is fine - so far. But if I take away the factor the reverse still remains.

This is the code for the historgam initially, with no factor. It should return to that if I add the factor and then take it away.

image

Thanks.

@Fidel365
Copy link
Contributor

@rdstern , the recent changes now fixes the issue and this ready for rechecking

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

@Vitalis95 and @Fidel365 this is great. What a happy outcome. @N-thony can you check the code and merge when you are happy?

Comment on lines 297 to 366
Private Sub UpdateParameter()
Dim strChangedTextFreq As String = ucrInputAddReorder.GetText()
clsRaesFunction.RemoveParameterByName("colour")
clsRaesFunction.RemoveParameterByName("y")
clsRaesFunction.RemoveParameterByName("fill")

clsForecatsInfreqValue.AddParameter("f", "as.factor(" & ucrFactorReceiver.GetVariableNames(False) & ")", iPosition:=0)

If rdoHistogram.Checked Then
If Not ucrFactorReceiver.IsEmpty Then
Select Case strChangedTextFreq
Case strAscending
clsForecatsReverse.AddParameter("f", clsRFunctionParameter:=clsForecatsInfreqValue, iPosition:=0)
clsRaesFunction.AddParameter("fill", clsRFunctionParameter:=clsForecatsReverse, iPosition:=0)
Case strDescending
clsRaesFunction.AddParameter("fill", clsRFunctionParameter:=clsForecatsInfreqValue, iPosition:=0)
Case strReverse
clsForecatsReverse.AddParameter("f", ucrFactorReceiver.GetVariableNames(False), iPosition:=0)
clsRaesFunction.AddParameter("fill", clsRFunctionParameter:=clsForecatsReverse, iPosition:=0)
Case strNone
clsRaesFunction.AddParameter("fill", ucrFactorReceiver.GetVariableNames(False), iPosition:=0)
End Select
End If
ElseIf rdoDensity_ridges.Checked Then
If Not ucrFactorReceiver.IsEmpty Then
If ucrChkRidges.Checked Then
Select Case strChangedTextFreq
Case strAscending
clsForecatsReverse.AddParameter("f", clsRFunctionParameter:=clsForecatsInfreqValue, iPosition:=0)
clsRaesFunction.AddParameter("y", clsRFunctionParameter:=clsForecatsReverse, iPosition:=0)
Case strDescending
clsRaesFunction.AddParameter("y", clsRFunctionParameter:=clsForecatsInfreqValue, iPosition:=0)
Case strReverse
clsForecatsReverse.AddParameter("f", ucrFactorReceiver.GetVariableNames(False), iPosition:=0)
clsRaesFunction.AddParameter("y", clsRFunctionParameter:=clsForecatsReverse, iPosition:=0)
Case strNone
clsRaesFunction.AddParameter("y", ucrFactorReceiver.GetVariableNames(False), iPosition:=0)
End Select
Else
Select Case strChangedTextFreq
Case strAscending
clsForecatsReverse.AddParameter("f", clsRFunctionParameter:=clsForecatsInfreqValue, iPosition:=0)
clsRaesFunction.AddParameter("colour", clsRFunctionParameter:=clsForecatsReverse, iPosition:=0)
Case strDescending
clsRaesFunction.AddParameter("colour", clsRFunctionParameter:=clsForecatsInfreqValue, iPosition:=0)
Case strReverse
clsForecatsReverse.AddParameter("f", ucrFactorReceiver.GetVariableNames(False), iPosition:=0)
clsRaesFunction.AddParameter("colour", clsRFunctionParameter:=clsForecatsReverse, iPosition:=0)
Case strNone
clsRaesFunction.AddParameter("colour", ucrFactorReceiver.GetVariableNames(False), iPosition:=0)
End Select
End If
End If
Else
If Not ucrFactorReceiver.IsEmpty Then
Select Case strChangedTextFreq
Case strAscending
clsForecatsReverse.AddParameter("f", clsRFunctionParameter:=clsForecatsInfreqValue, iPosition:=0)
clsRaesFunction.AddParameter("colour", clsRFunctionParameter:=clsForecatsReverse, iPosition:=0)
Case strDescending
clsRaesFunction.AddParameter("colour", clsRFunctionParameter:=clsForecatsInfreqValue, iPosition:=0)
Case strReverse
clsForecatsReverse.AddParameter("f", ucrFactorReceiver.GetVariableNames(False), iPosition:=0)
clsRaesFunction.AddParameter("colour", clsRFunctionParameter:=clsForecatsReverse, iPosition:=0)
Case strNone
clsRaesFunction.AddParameter("colour", ucrFactorReceiver.GetVariableNames(False), iPosition:=0)
End Select
End If
End If
End Sub
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you don't need to copy paste the same code to the three options as it apply to all of them directly. Just one block will be enough?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@N-thony , the ucrFactorReceiver passes different parameters on the three buttons i.e histogram , it passes fill, density it passes color and y depending on the Ridges checkbox and underpolygon it passes colour

Is there another way I can do that?

instat/dlgHistogram.vb Outdated Show resolved Hide resolved
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.

Still looks good

@N-thony N-thony merged commit f8b8462 into IDEMSInternational:master Sep 22, 2023
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.

Add controls to histogram plots just as the bar charts
4 participants