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

Click 8.0 Custom Parameter Types and convert() #1898

Closed
makermelissa opened this issue May 14, 2021 · 4 comments
Closed

Click 8.0 Custom Parameter Types and convert() #1898

makermelissa opened this issue May 14, 2021 · 4 comments
Labels
Milestone

Comments

@makermelissa
Copy link

It appears the actual values are coming through rather than strings of the values which is causing some unexpected results.

@makermelissa
Copy link
Author

This code works fine in 7.1.2, but fails in 8.0.0. Documentation doesn't note that the values should have changed. I don't have the exact error anymore, but value was an int, so it choked on value.lower() since you can't call lower() on an int.

class BasedIntOrNoneParamType(click.ParamType):
    name = 'Int or None'

    def convert(self, value, param, ctx):
        try:
            if value.lower() == 'none':
                return 'none'
            return int_as_text_to_int(value)
        except nRFException:
            self.fail('%s is not a valid integer' % value, param, ctx)

BASED_INT_OR_NONE = BasedIntOrNoneParamType()

The code is from here:
https://github.com/adafruit/Adafruit_nRF52_nrfutil/blob/master/nordicsemi/__main__.py#L59-L70

@davidism
Copy link
Member

The value processing did change somewhat, and converters may get values that are already the correct type in situations such as using a default value or passing values from Python code instead of the command line. The reason for this change is so that type casting is applied consistently throughout the processing pipeline, regardless of how the command is called or where the value comes from.

My guess is that you have a default=10 or something, and that's getting passed through. The relevant changelog line is probably this one (admittedly there's a lot to sift through this time), or one of the others from #1687:

When getting the value for a parameter, the default is tried in the same section as other sources to ensure consistent processing. #1649

You can adapt your custom type with something like this:

def convert(self, value, param, ctx):
    if isinstance(value, int):
        return value

If this isn't the issue, can you post a self-contained example and the error you get?

@JNRowe
Copy link

JNRowe commented May 17, 2021

I'm also experiencing the same issue with version 8.0. While the isinstance() check will resolve it for me, I'm just looking to check the repeated calls to convert() are the expected behaviour now before I make the changes. For example, using the custom types example will break if a default is given:

@click.command()
@click.option('-n', type=BASED_INT, default=10)
def main(n):
    pass

fails with Error: Invalid value for '-n': expected string for int() conversion, got 10 of type int. If this is expected behaviour, perhaps the docs need updating?

@makermelissa
Copy link
Author

Thanks. I think I can get past my particular issue by wrapping the value inside of str().

@davidism davidism added the docs label May 18, 2021
@davidism davidism added this to the 8.0.1 milestone May 19, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants