[samigo-team] Regression: number formats of Numeric answers

[Earle Nietzel]

Hi everyone,

After reviewing this issue I believe this should be good for 2.9. It's
handling the comma's in the same manner as they were in 2.8, agreed
that this solution may not be ideal see Davids minor rant below, but
it is no worse than the way it was done before!

So for the interests in moving forward :)

+1 for inclusion

Earle

On Fri, Sep 7, 2012 at 5:07 PM, Karen Tsao <ktsao at stanford.edu> wrote:
> Hi Neal,
>
> Do you mean reverting the whole feature or just the fix that I added in
> yesterday? I believe it will take lots effort to revert this feature. If you
> mean reverting my fix, I think if we can perform tests to cover different
> scenarios, it can go in. Especially, Earle has reviewed the code and fixed
> my typo  :)
>
> And then if we want to go for a better solution, we can anyway do it in
> 2.9.1. What do you think?
>
> Earle,
>
> Thanks a lot for reviewing it and correct my typo.
>
> Karen
>
>
>
> On Fri, Sep 7, 2012 at 9:29 AM, Neal Caidin <nealcaidin at sakaifoundation.org>
> wrote:
>>
>> Should we consider getting this done the "right" way and holding it back
>> from the 2.9.0 release, maybe target 2.9.1 , or will this be good enough?
>>
>> For the Test Fest, do we have any test scripts on this so others can also
>> try and break it?
>>
>> Thanks,
>> Neal
>>
>> On Sep 7, 2012, at 3:31 AM, David Horwitz <david.horwitz at uct.ac.za> wrote:
>>
>> I will do some testing on this.
>>
>> *minor rant*
>> I am concerned about some of the code we're accepting and the consequences
>> we see. There are a number of issues with this code:
>>
>> - The functions are not documented. What does FinQuestionValidator and its
>> methods do? What is the expected behaviour?
>> - The lack of unit test coverage. While the architecture of SAM makes it
>> dicficult (or imposible) to cover anything there is no reason why utility
>> methods like this shouldn't be.
>> - The answers are validated in different ways by different parts of the
>> code - this is asking for trouble.
>> *end minor rant*
>>
>> D
>> On 09/07/2012 01:34 AM, Karen Tsao wrote:
>>
>> I made a fix which basically adds back the related lines that have been
>> removed. Now the comma works as period. As there is not much time, I only
>> did some simple tests. Can we get some QA help to perform detailed
>> regression tests as SAM-1221 contains couple of different scenarios.
>>
>> Thanks,
>> Karen
>>
>> On Thu, Sep 6, 2012 at 10:50 AM, David Horowitz <david.horwitz at uct.ac.za>
>> wrote:
>>>
>>> Unfortunately didn't have a chance to look into it today. The fix I had
>>> seemed to break the si notation reading. Hence the unit tests I put in trunk
>>>
>>> Sent from my iPad
>>>
>>> On 06 Sep 2012, at 7:02 PM, Karen Tsao <ktsao at stanford.edu> wrote:
>>>
>>> Hi David,
>>>
>>> Any progress in https://jira.sakaiproject.org/browse/SAM-1793? As Mathew
>>> pointed out, your fix is a better solution, can you let me know why you
>>> reverted it? I can take over the remaining work if we are on the right
>>> track.
>>>
>>> Thanks,
>>> Karen
>>>
>>> On Wed, Sep 5, 2012 at 10:22 AM, David Horowitz <david.horwitz at uct.ac.za>
>>> wrote:
>>>>
>>>> Hi Karen,
>>>>
>>>> I committed a change set against 1221 that includes a unit test for the
>>>> common cases and 1 for the broken case commented out. The change set I
>>>> committed to uct msub seems to break other things.
>>>>
>>>> I will check in the morning
>>>>
>>>> D
>>>>
>>>> Sent from my iPad
>>>>
>>>> On 05 Sep 2012, at 7:03 PM, Karen Tsao <ktsao at stanford.edu> wrote:
>>>>
>>>> Hi David,
>>>>
>>>> I see what you mean. I will look into this now. But as SAM-1221 is
>>>> contributed by UPV, I will contact them to see if they can provide a fix
>>>> sooner.
>>>>
>>>> By the way, do you have any test case set because SAM-1221 can have many
>>>> different combinations. I want to be able to cover all of them.
>>>>
>>>> Thanks,
>>>> Karen
>>>>
>>>> On Wed, Sep 5, 2012 at 4:51 AM, David Horwitz <david.horwitz at uct.ac.za>
>>>> wrote:
>>>>>
>>>>> Hi All,
>>>>>
>>>>> I seem to have found a serious regression in 2.9:
>>>>>
>>>>> https://jira.sakaiproject.org/browse/SAM-1793
>>>>>
>>>>> it seems to have been introduced by SAM-122 (support for scientific
>>>>> notation)
>>>>>
>>>>> In 2.8 and prior either a comma or a full stop could be used to denote
>>>>> a
>>>>> decimal separator. The UI still lists the comma as a valid separator
>>>>> outside of scientific notation. However gradingService.validate only
>>>>> accepts a full stop.
>>>>>
>>>>> In the case of an upgraded system it means that any students answers
>>>>> with a comma will lead to an unhandled exception.
>>>>>
>>>>> Do we need a complex upgrade to fix all answers (and a fix to the UI
>>>>> docs) or is the method buggy?
>>>>>
>>>>> Regards
>>>>>
>>>>> David
>>>>> _______________________________________________
>>>>> samigo-team mailing list
>>>>> samigo-team at collab.sakaiproject.org
>>>>> http://collab.sakaiproject.org/mailman/listinfo/samigo-team
>>>>
>>>>
>>>
>>
>>
>> _______________________________________________
>> samigo-team mailing list
>> samigo-team at collab.sakaiproject.org
>> http://collab.sakaiproject.org/mailman/listinfo/samigo-team
>>
>>
>