lukew3 / mathgenerator

A math problem generator, created for the purpose of giving self-studying students and teaching organizations the means to easily get access to high-quality, generated math problems to suit their needs.
https://lukew3.github.io/mathgenerator
MIT License
681 stars 176 forks source link

decimal_to_roman_numerals function issue #437

Open autosaver opened 8 months ago

autosaver commented 8 months ago

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

Getting 0 every time

The number $0$ in Roman Numerals is:
MCDII
hamzmu commented 8 months ago

looks good, works on my end! image

lukew3 commented 8 months ago

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

itssammurphy commented 8 months ago

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

autosaver commented 8 months ago

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 commented 8 months ago

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