Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

decimal_to_roman_numerals function issue #437

Open
autosaver opened this issue Dec 2, 2023 · 5 comments
Open

decimal_to_roman_numerals function issue #437

autosaver opened this issue Dec 2, 2023 · 5 comments

Comments

@autosaver
Copy link

problem = f"The number ${x_copy}$ in roman numerals is: "

Getting 0 every time

The number $0$ in Roman Numerals is:
MCDII
@hamzmu
Copy link

hamzmu commented Dec 9, 2023

looks good, works on my end!
image

@lukew3
Copy link
Owner

lukew3 commented Dec 9, 2023

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) ?

def decimal_to_roman_numerals(max_decimal=4000):

@itssammurphy
Copy link

@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
Copy link
Author

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) ?

def decimal_to_roman_numerals(max_decimal=4000):

Actually I was calling it directly without variable

@autosaver
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants