feat: python 3.12 upgrade

[ichintanjoshi]

Description

This PR contains changes for upgrading python 3.8 to python 3.12.
Changes include :-

• Dependencies and version upgrades
• Extra check for factorial function to get only integers
  • This is done because evaluator function in calc.py file will convert all numbers to float
  • And python 3.12 will not allow math.factorial to calculate float values and will raise an error ref
• Removal of float numbers for factorial function tests (Same explanation as above)

Done as a part of this issue openedx/public-engineering#233

[openedx-webhooks]

Thanks for the pull request, @ichintanjoshi! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

• supporting documentation
• Open edX discussion forum threads
• timeline information ("this must be merged by XX date", and why that is)
• partner information ("this is a course on edx.org")
• any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

[feanil]

Why were these assertions removed?

[ichintanjoshi]

@feanil python 3.12 will not allow math.factorial to calculate float values and will raise an error ref

So earlier if we sent integers like 1.0 it'll work but if we sent float 1.5 it used to give value error
but now if we send integers like 1.0 it'll give error, and because of that I wrote extra code to check if it's factorial we'll convert all incoming numbers to integers, because calc.py has evaluator function which converts all numbers to floats. And if we send it 0.5 it'll just take it as 0 and the test won't work..

[feanil]

@ichintanjoshi Right, but I'm confused, it sounds like you're changing the behavior. This test confirms that it does raise a value error which is what we want. math.factorial raised a value error in 3.8 as well so we shouldn't need to change anything here. Can you share the output of the failing test that you were fixing?

[ichintanjoshi]

@feanil, yes,

I'll try to explain my changes.

Python 3.8

The value we were sending from here test_calc.py

Were being sent as 0.0 or 1.0 5.0 etc and that was allowed to be calculated in python 3.8 factorial.

Python 3.12

While the values being sent are still the same i.e. 0.0 or 1.0, it's no longer allowed in python 3.12 to pass them like that in float, we need to pass them as 0 or 1 like integers, as a result I made this change, to convert these values to integer if the function being called is integer change ref

What happens now is 0.5 will be parsed as 0, because I am now converting things passed for factorials as integers. Thus I removed it.

As for the error output if we don't make changes, it is something like this.

======================================================================
ERROR: test_other_functions (tests.test_calc.EvaluatorTest.test_other_functions)
Test the non-trig functions provided in calc.py
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/chintan/openedx-repos/openedx-calc_old/tests/test_calc.py", line 347, in test_other_functions
    self.assert_function_values('fact', fact_inputs, fact_values)
  File "/home/chintan/openedx-repos/openedx-calc_old/tests/test_calc.py", line 168, in assert_function_values
    result = calc.evaluator({}, {}, input_str)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/chintan/openedx-repos/openedx-calc_old/calc/calc.py", line 281, in evaluator
    return math_interpreter.reduce_tree(evaluate_actions)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/chintan/openedx-repos/openedx-calc_old/calc/calc.py", line 454, in reduce_tree
    return handle_node(self.tree)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/home/chintan/openedx-repos/openedx-calc_old/calc/calc.py", line 450, in handle_node
    handled_kids = [handle_node(k) for k in node]
                    ^^^^^^^^^^^^^^
  File "/home/chintan/openedx-repos/openedx-calc_old/calc/calc.py", line 450, in handle_node
    handled_kids = [handle_node(k) for k in node]
                    ^^^^^^^^^^^^^^
  File "/home/chintan/openedx-repos/openedx-calc_old/calc/calc.py", line 450, in handle_node
    handled_kids = [handle_node(k) for k in node]
                    ^^^^^^^^^^^^^^
  [Previous line repeated 2 more times]
  File "/home/chintan/openedx-repos/openedx-calc_old/calc/calc.py", line 451, in handle_node
    return action(handled_kids)
           ^^^^^^^^^^^^^^^^^^^^
  File "/home/chintan/openedx-repos/openedx-calc_old/calc/calc.py", line 268, in eval_function
    return all_functions[casify(x[0])](x[1])
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: 'float' object cannot be interpreted as an integer

[feanil]

@ichintanjoshi I dug into it a little deeper, based on your stack trace, it looks like the error is happening with the call on line 347 and not on the line that you removed. I don't think the fix you put in place is the right one because we don't want to coerce actual floats to integers as that will fail to raise errors when it correctly should.

I think the real problem is with the eval_number function which coerces all numbers to floats. I think that function needs to be smarter and if there are no decimals in the number then it needs to return integers instead of numbers.

[ichintanjoshi]

Yes, that was the issue, Okay I'll take a look into it if I can fix it.

[feanil]

@ichintanjoshi so it looks like there is no version of numpy that supports both Python 3.8 and 3.12 so for edx-platform and its librararies(including this one) we should aim to get it working on 3.8 and 3.11 instead. For the dependencies, we should still compile them with 3.8 but run the test with both 3.8 and 3.11 so that we can be confident that it will work with both.

[feanil]

@ichintanjoshi I dug into it a little deeper, based on your stack trace, it looks like the error is happening with the call on line 347 and not on the line that you removed. I don't think the fix you put in place is the right one because we don't want to coerce actual floats to integers as that will fail to raise errors when it correctly should.

I think the real problem is with the eval_number function which coerces all numbers to floats. I think that function needs to be smarter and if there are no decimals in the number then it needs to return integers instead of numbers.

[ichintanjoshi]

Hi @feanil I checked it's not through eval_number, I did make it to return int only, but that fails with the same error. In the same place, I think the number is getting it's float value from somewhere else.

For example here is the code that I tried it with

def eval_number(parse_result):
    """
    Create a float out of its string parts.

    e.g. [ '7.13', 'e', '3' ] ->  7130
    Calls super_float above.

    or return int, if numbers are int only
    """
    joined_result = "".join(parse_result)
    try:
        if isinstance(eval(joined_result), numbers.Number):
            if math.ceil(eval(joined_result)) == eval(joined_result):
                return math.ceil(eval(joined_result))
    except Exception as e:
        pass


    return super_float(joined_result)

[openedx-webhooks]

@ichintanjoshi Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.