Fractions functionality issue #395

[SCMusson]

Targetting issue #395.
Dunder methods for all Fractions functions.
The Fraction functions are still there, I don't know if you want to temporarily keep them for backwards compatibility or to be replaced entirely with dunder methods.

Also created Fraction.norm and Fraction.ceil class methods which could replace ceil_fraction and norm_fraction.

[nielstron]

I would remove the old methods.

One plan for this was also that it is merged after #397, such that add mul etc have support for Union[int, Fraction].

[SCMusson]

That's fine. #397 is pretty much ready to go. I'm happy to come back to this when its merged.

[SCMusson]

I had to modify a few different things so that Self in Unions was accepted.

Currently running something like:

from opshin.tests.utils import eval_uplc
from opshin.std.fractions import Fraction
a = Fraction(5,7)
b = Fraction(11,13)

source_code = """
from opshin.std.fractions import *
from typing import Dict, List, Union

def validator(a: Fraction, b: Union[Fraction, int]) -> Fraction:
    return a + b
"""
ret = eval_uplc(source_code, a,b)

Works but running eval_uplc(source_code, a, 5)

Returns the following error:

Traceback (most recent call last):
  File "/home/wppj21/Workshop/opshin/test_class.py", line 121, in <module>
    ret = eval_uplc(source_code, a, 5)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/wppj21/Workshop/opshin/opshin/tests/utils.py", line 35, in eval_uplc
    raise ret
  File "/home/wppj21/miniconda3/envs/opshin/lib/python3.11/site-packages/uplc/machine.py", line 111, in eval
    stack.append(self.return_compute(step.context, step.value))
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/wppj21/miniconda3/envs/opshin/lib/python3.11/site-packages/uplc/machine.py", line 171, in return_compute
    return self.apply_evaluate(context.ctx, context.val, value)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/wppj21/miniconda3/envs/opshin/lib/python3.11/site-packages/uplc/machine.py", line 210, in apply_evaluate
    res = eval_fun(*arguments)
          ^^^^^^^^^^^^^^^^^^^^
  File "/home/wppj21/miniconda3/envs/opshin/lib/python3.11/site-packages/uplc/ast.py", line 745, in wrapped_fun
    assert isinstance(
           ^^^^^^^^^^^
AssertionError: Argument 0 has invalid type, expected type <class 'uplc.ast.BuiltinInteger'> got <class 'uplc.ast.PlutusInteger'> (PlutusInteger(value=5))

Is this an bug in uplc or have I made an error somewhere?

[nielstron]

There is a bug in OpShin, likely something we overlooked in #397.

When something is a Union[x,y] it is PlutusData. However for int and other primitive types I usually assume they are unwrapped into builtin data. Here there must be some missing unwrapping of int after casting it via isinstance. Note that addition can not be done with PlutusData and requires unwrapping via UInt before.

[nielstron]

Try

def validator(x: Union[int, A]):
  if isinstance(x, int):
     print(x+2)

[nielstron]

There is likely also another edge case in #397

def validator(x: int):
   y : Union[int, A] = x
   if isinstance(y, int):
     print("foo")

will likely crash

[SCMusson]

Is CI not working at the moment?

[nielstron]

Great, thanks for the update. I think Travis is currently broken, I wanted to migrate to Github actions anyways and am preparing #406 for this. Will merge it into this branch to check that the tests are now still passing.

[nielstron]

It looks like Self is a 3.11 only feature of typing. In previous python versions this is instead solved using literal strings like "Fraction".

[SCMusson]

Ah, I'll rewrite them all as literal strings then.

[coveralls]

coverage: 89.428% (+0.02%) from 89.406%
when pulling 303821b on SCMusson:feat_frac_dunder
into 6f99124 on OpShin:dev.

[nielstron]

The fraction code looks great now. Unfortunately I still found (and added) 2 failing cases for #397.

[SCMusson]

I'll get to work on those

[SCMusson]

Thank you for those test cases, the first one I really should have thought of myself but I wouldn't have thought of the test_Union_builtin_cast_direct. I had to modify them so that the tests were correct so It might be worth you checking to make sure I didn't break the test.

[nielstron]

Your modifications of if-expressions made me think... caught another bug :)

[nielstron]

Sorry about the never ending new crashing examples... the last ones added are out of scope though (since not even properly implemented for unions of plutus data), I created #408 dedicated for them. also its not a problem if an expression does not compile.

[nielstron]

Any update on this? :)

[SCMusson]

Apologies, I almost forgot I was supposed to be working on this. I've got the IfExp working now

[nielstron]

Great, thank you! This will be part of the next version 0.24.0. Please submit your wallet address for the bug bounty :)

[SCMusson]

addr1q84dsy6ckn3gplf6kenwvr8mspecdlxu2mq3aj5kn02uthd93vtsr3yl4f3553sxfx7wzx58cx5pnhekvtlgqf55lw9sl4vwdy

[SCMusson]

If this also signals the completion of #397 the same address can be used for the payout of that bounty too.

[nielstron]

Paid out both #367 and #395
https://adastat.net/transactions/c62300dc553ae877376c7bb281997faf453cd5776c16db4e75b14584efbc76ce