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

Parameter values of type int should also work with type float #67

Closed
siliconlad opened this issue Apr 10, 2024 · 3 comments · Fixed by #79
Closed

Parameter values of type int should also work with type float #67

siliconlad opened this issue Apr 10, 2024 · 3 comments · Fixed by #79
Assignees
Labels
buglet 🐛 Annoying thopugh doesn't completely inhibit use

Comments

@siliconlad
Copy link
Member

siliconlad commented Apr 10, 2024

When calling LoadGPTEnabled, I have a situation where I'm passing 2 into a parameter of type float and it errors out when validating the type.

@siliconlad siliconlad added the buglet 🐛 Annoying thopugh doesn't completely inhibit use label Apr 10, 2024
@siliconlad siliconlad self-assigned this Apr 10, 2024
@siliconlad
Copy link
Member Author

Perhaps an extra flag like coerce_type or something.

@lorenzocelli
Copy link
Contributor

The first fix that comes to mind is (in type_schema.py:142):

class ValueTypeSchema(TypeSchema):
    # ...

    def validate(self, value) -> bool:
        try:
            self.type(value)
            return True # Return true if the value can be converted to the type
        except ValueError:
            return False

But this might be too permissive. For example, I think every value can be converted to boolean.

Otherwise we could have a special case for float and int:

def validate(self, value) -> bool:
    return (self.type is float and type(value) is int) or self.type == type(value)

But this might not work in other cases, if there are any. It depends on Python rules for implicit conversion. Reading online I did not find any other cases of implicit cast, so the special check might not be that bad.

@siliconlad siliconlad linked a pull request Apr 30, 2024 that will close this issue
@siliconlad
Copy link
Member Author

Allowing implicit conversion from int to float looks good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
buglet 🐛 Annoying thopugh doesn't completely inhibit use
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants