mathgenerator icon indicating copy to clipboard operation
mathgenerator copied to clipboard

decimal_to_roman_numerals function issue

Open autosaver opened this issue 1 year ago • 5 comments

https://github.com/lukew3/mathgenerator/blob/6e11124a1cd3840dfc1b78f8f3cdea5b02f65744/mathgenerator/misc.py#L229

Getting 0 every time

The number $0$ in Roman Numerals is:
MCDII

autosaver avatar Dec 02 '23 17:12 autosaver

looks good, works on my end! image

hamzmu avatar Dec 09 '23 03:12 hamzmu

Getting 0 every time

@autosaver Are you overriding the max_decimal kwarg with decimal_to_roman_numerals(max_decimal=0) or decimal_to_roman_numerals(0) ? https://github.com/lukew3/mathgenerator/blob/6e11124a1cd3840dfc1b78f8f3cdea5b02f65744/mathgenerator/misc.py#L193

lukew3 avatar Dec 09 '23 04:12 lukew3

@lukew3 I edited the gen_func() associated with decimal_to_roman_numerals (in the site-packages folder where mathgenerator got pip installed) and was able to resolve the issue. image In the above snippet, I added a line (as you did in misc.py)

x_copy = x

image Then in the above snippet, I changed the f-string in the problem statement to reflect the correct variable name.

These fixes corrected the issue, but I'm not sure how to reflect these in my fork to then pull request it as this file isn't in the repository but in the pip installation? I am new to open source contribution so please let me know if I have misread something somewhere or missed a file that's in the fork.

itssammurphy avatar Dec 19 '23 12:12 itssammurphy

Getting 0 every time

@autosaver Are you overriding the max_decimal kwarg with decimal_to_roman_numerals(max_decimal=0) or decimal_to_roman_numerals(0) ?

https://github.com/lukew3/mathgenerator/blob/6e11124a1cd3840dfc1b78f8f3cdea5b02f65744/mathgenerator/misc.py#L193

Actually I was calling it directly without variable

autosaver avatar Dec 19 '23 14:12 autosaver

It seems its using :

from ...generator import Generator
import random
import math


def gen_func(maxDecimal=4000):
    x = random.randint(0, maxDecimal)
    roman_dict = {
        1: "I",
        5: "V",
        10: "X",
        50: "L",
        100: "C",
        500: "D",
        1000: "M"
    }
    div = 1
    while x >= div:
        div *= 10
    div /= 10
    solution = ""
    while x:
        last_value = int(x / div)
        if last_value <= 3:
            solution += (roman_dict[div] * last_value)
        elif last_value == 4:
            solution += (roman_dict[div] + roman_dict[div * 5])
        elif 5 <= last_value <= 8:
            solution += (roman_dict[div * 5] + (roman_dict[div] * (last_value - 5)))
        elif last_value == 9:
            solution += (roman_dict[div] + roman_dict[div * 10])
        x = math.floor(x % div)
        div /= 10

    problem = f"The number ${x}$ in Roman Numerals is: "
    return problem, f'${solution}$'


decimal_to_roman_numerals = Generator("Converts decimal to Roman Numerals", 85,
                                      gen_func,
                                      ["maxDecimal=4000"])

Trying to upgrade mathgenerator

autosaver avatar Dec 22 '23 16:12 autosaver