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

[Neal Caidin]

Hi All,

I don't know enough technically to comment (and maybe I shouldn't have to begin with). It was really just a question that came up in my brain based on Horowitz's email, his minor rant. Thanks for your responses.

-- Neal
 


On Sep 7, 2012, at 5:50 PM, Earle Nietzel <earle.nietzel at gmail.com> wrote:

> 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
>>> 
>>> 
>>