openfun / ralph

:gear: Ralph, the ultimate Learning Record Store (and more!) for your learning analytics
https://openfun.github.io/ralph/
MIT License
36 stars 15 forks source link

Scaled score is typed as int #612

Closed lmaheo closed 1 month ago

lmaheo commented 1 month ago

Bug Report

Expected behavior/code I would expect statement.result.score.scaled to be of type float generally. Does the xAPI documentation specify that it should be an int? int casting often replaces scaled scores with a 0...

Actual Behavior Currently statement.result.score.scaled is typed as int, so scores between 0 and 1 get crushed to 0.

Steps to Reproduce

  1. Run
    from ralph.models.xapi.base.results import BaseXapiResultScore
    print(BaseXapiResultScore.parse_obj({
    "raw": 1,
    "min": 0,
    "max": 2,
    "scaled": 0.5,
    }))
  2. See that the scaled value is 0 in this case, instead of 0.5 or Decimal('0.5') or something to the same effect.

Environment

Possible Solution

I would replace the line here with : scaled: Optional[Annotated[int, Field(ge=-1, le=1)]] = None scaled: Optional[Annotated[float, Field(ge=-1, le=1)]] = None

wilbrdt commented 1 month ago

It's an error indeed! The xAPI documentation specifies that scaled is a decimal number between 0 and 1. I see that you are using Ralph version 3.9.0. Are you using Pydantic v1? Asking to know if upgrading to a latest Ralph v5 version with a fix would be a problem for you.

lmaheo commented 1 month ago

I am indeed using pydantic v1, but that's OK! I would welcome the fix, even if the migration to pydantic v2 is not possible right now :) By the way, to make sure we're not introducing another error: the spec mentions Decimal number between -1 and 1, inclusive. So the current bounds are OK, it's only the typing that's wrong.

wilbrdt commented 1 month ago

Whoops yes good catch! I mistyped and was not planning to change the bounds :)