format date columns issue partialy solved#9866
format date columns issue partialy solved#9866ARMSTRONGOPONDO wants to merge 6 commits intoIDEMSInternational:masterfrom
Conversation
|
@ARMSTRONGOPONDO please remember to write the pull request as (fixes partly #4282) and for any other issues you will attempt in the future |
|
@ARMSTRONGOPONDO great. Is this ready for review or are you still working on it? |
|
@ARMSTRONGOPONDO it is now 2 weeks since the @lilyclements question above. You have been silent and I wonder what you are working on, if not this?
So I tried typing into the receiver, but can't get Ok to be enabled. So I am unable to check if you have started on the R code. This all seems remarkably little so far. Maybe it was 30 minutes of work? Please let us know when you have made some progress and reply before then to say it is not yet ready for testing. |
lilyclements
left a comment
There was a problem hiding this comment.
@ARMSTRONGOPONDO this is great! Really well done!! Working perfectly. The only changes are in the input text itself.
There are two small changes outlined below
|
@ARMSTRONGOPONDO Nice one for adding it in |
|
@ARMSTRONGOPONDO just had a look. Can you position it here instead:
Let me know when that's done. Otherwise this is looking great! nice job! |
|
@ARMSTRONGOPONDO and @lilyclements, here are some suggestions:
It is the control at the bottom namely New Column Name, up to the Position button. b) A smaller change, could you label your help button as c) Could you delete - well move really - the examples section from here. That will makie that control neater, because it won't need the scroll bar:
d) Could you please add all those examples at the top of the pull down, shown in your current dialog above. You could include the brackets too, e.g. |
|
@ARMSTRONGOPONDO this is looking good. Are you able to work on @rdstern's suggestions? |
|
Am currently working on it @lilyclements |
|
I've implemented all the other requirements, but after implementing the new column name, I continue to receive this error. Could you suggest a way to solve it |
|
@ARMSTRONGOPONDO can you send up your most recent changes so I can have a look into this error? Looks like an issue in the R code so something for my end to fix :) |
- Introduced `ucrFormatNew` control for specifying new column names in `dlgUseDate`. - Updated help button text to "R Help" for clarity. - Improved `ComboBoxnewformat` with specific date format examples and tooltips. - Enhanced `TestOKEnabled` method to check `ucrFormatNew` completeness.
|
@lilyclements here are the implemented changes that ive pushed today ( I had a problem with my laptop) This pull request introduces significant improvements to the date formatting functionality in the Date Formatting Enhancements:
Usability Improvements in Crop Dialog: |
lilyclements
left a comment
There was a problem hiding this comment.
@ARMSTRONGOPONDO good news! You've done it - and you've done too much if anythign!
You are running, say:
format_date <- data_book$split_date(data_name="ghana", col_name="date", format_date=TRUE, format_string="%b %d", new_col_name="format_date", s_start_month=1)
data_book$add_columns_to_data(data_name="ghana", col_name="format_date", col_data=format_date, before=FALSE, adjacent_column="date")But we just want to run:
data_book$split_date(data_name="ghana", col_name="date", format_date=TRUE, format_string="%b %d", new_col_name="format_date", s_start_month=1)That is, we want to not have the assign to, and not to have the $add_columns_to_data.
I've outlined below how that can be achieved (my 1-6 points below).
I've also given other small bits and pieces to help improve this code.
Nice one!
|
|
||
|
|
There was a problem hiding this comment.
Just because I'm picky, can you only have at most 1 empty line here (and elsewhere)
| 'Save control | ||
| ucrFormatNew.SetSaveTypeAsColumn() | ||
| ucrFormatNew.SetDataFrameSelector(ucrSelectorUseDate.ucrAvailableDataFrames) | ||
| ucrFormatNew.SetLabelText("New Column Name:") | ||
| ucrFormatNew.SetIsComboBox() | ||
| ucrFormatNew.SetPrefix("format_date") | ||
| ucrFormatNew.setLinkedReceiver(ucrReceiverUseDate) | ||
|
|
There was a problem hiding this comment.
We do not want a save control. I can completely see why you would think we do. But, the thing is, the save control is too clever and automatically runs this line of data_book$add_columns_to_data.
But, we don't want to run that here.
Instead, can you:
- Remove the save control
- Add in a simple Label with the text "New Column Name:"
- Add in a ucrInput.
- The value of that ucrInput can be
format_dateby default - That ucrInput is then the parameter value to the
new_col_nameparameter. - This should only be visible when the
rdoFormatColumnis checked.
So, to 3-5, for our ucrInput, to set this up, we would have like:
ucrNEWINPUTNAME.SetParameter(New RParameter("new_col_name", 4))
ucrNEWINPUTNAME.SetText("format_date")To 6., we can make thing visible/invisible using .AddToLinkedControls.
For example, look in dlgTransformText. We run there:
ucrPnlOperation.AddToLinkedControls(ucrInputSeparator, {rdoWords}, bNewLinkedAddRemoveParameter:=True, bNewLinkedHideIfParameterMissing:=True)This means that for ucrPnlOperation, when the panel is on rdoWords the ucrInputSeparator is made visible.
Can you see how? And can you see how we would/could then apply this to your case here using your ucrPnl, your rdoFormatColumns and your new ucrNEWINPUTNAME?
| Private Sub GroupBoxSettings() | ||
| If rdoUseColumn.Checked Then | ||
| Panelusemode.Visible = True | ||
| Panelformatmode.Visible = False | ||
| grpUseColumnnewfrm.Visible = False | ||
| grpfrmdescrp.Visible = False | ||
| clsDefaultFunction.AddParameter("format_date", "FALSE", iPosition:=4) | ||
| clsDefaultFunction.RemoveParameterByName("format_string") | ||
| clsDefaultFunction.RemoveParameterByName("new_col_name") | ||
| cmdHelp.Visible = False | ||
| ElseIf rdoFormatColumn.Checked Then | ||
| Panelusemode.Visible = False | ||
| Panelformatmode.Visible = True | ||
| grpUseColumnnewfrm.Visible = True | ||
| grpfrmdescrp.Visible = True | ||
| clsDefaultFunction.AddParameter("format_date", "TRUE", iPosition:=4) | ||
| If Not String.IsNullOrEmpty(ComboBoxnewformat.Text) Then | ||
| clsDefaultFunction.AddParameter("format_string", Chr(34) & ComboBoxnewformat.Text & Chr(34), iPosition:=5) | ||
| End If | ||
| clsDefaultFunction.AddParameter("new_col_name", Chr(34) & ucrFormatNew.GetText() & Chr(34), iPosition:=6) | ||
| cmdHelp.Visible = True | ||
| End If |
There was a problem hiding this comment.
We can remove some of this. So, we can remove our stuff that runs .AddParameter("format_date", ...) bits. See the next comment below.
Similarly, we can remove clsDefaultFunction.AddParameter("new_col_name", ... too by changing the set up to how it's outlined above
| ucrPnluseformat.AddRadioButton(rdoUseColumn) | ||
| ucrPnluseformat.AddRadioButton(rdoFormatColumn) |
There was a problem hiding this comment.
To add/remove our format_date parameter, we can instead change the set up in our Initialise Dialog here.
So, we want to say in our Initialise that if you click on rdoUse, then we add format_date = FALSE, and if you click on rdoFormatColumns, our format_date = TRUE.
If we again look at dlgTransformText, we can see the line:
ucrPnlOperation.AddToLinkedControls(ucrPnlPad, {rdoTrim}, bNewLinkedHideIfParameterMissing:=True)
ucrPnlPad.AddRadioButton(rdoLeftPad)
ucrPnlPad.AddRadioButton(rdoRightPad)
ucrPnlPad.AddParameterValuesCondition(rdoLeftPad, "side", Chr(34) & "left" & Chr(34))
ucrPnlPad.AddParameterValuesCondition(rdoRightPad, "side", Chr(34) & "right" & Chr(34))This is very simple! It is simply saying, if you click on the rdoRightPad, add in side = "right" as a parameter value.
If you click on the rdoLeftPad, add in side = "left" as a parameter value.
So we would just want to have
ucrPnluseformat.AddParameterValuesCondition(rdoUseColumn, "format_date", "FALSE")
ucrPnluseformat.AddParameterValuesCondition(rdoFormatColumn, "format_date", "TRUE"`)| ucrBase.clsRsyntax.SetBaseRFunction(clsDefaultFunction) | ||
|
|
||
|
|
||
| ucrPnluseformat.SetRCode(clsDefaultFunction) |
There was a problem hiding this comment.
Anything with .SetRCode is always run in SetRCodeForControls sub. This should be removed from here
| Panelusemode.Visible = True | ||
| Panelformatmode.Visible = False |
There was a problem hiding this comment.
Out of interest, what does this do?
| End If | ||
|
|
||
|
|
||
| ucrPnluseformat.SetRCode(clsDefaultFunction, bReset:=False) |
There was a problem hiding this comment.
Anything with .SetRCode is always run in SetRCodeForControls sub. This should be removed from here
| If rdoUseColumn.Checked Then | ||
| End If |
| If rdoFormatColumn.Checked Then | ||
| End If |
| Private Sub Controls_ControlValueChanged() Handles rdoUseColumn.CheckedChanged, rdoFormatColumn.CheckedChanged, ComboBoxnewformat.TextChanged, ucrReceiverUseDate.ControlContentsChanged | ||
| GroupBoxSettings() | ||
| End Sub |
There was a problem hiding this comment.
For this, you want to have .ControlContentsChanged for them all. So, instead of using rdoUseColumn or rdoFormatColumn, you should use the ucrPnl....
Whenever you have a ucr, it's very simply ControlContentsChanged, or ControlValueChanged - much simpler!









@Ag-Derek
@lilyclements
@rdstern
added a format tab radio button the new column and format description dialog #4282