Add coercion/normalization of Choice values

[altendky]

I want my CLI to be case insensitive for certain Choice options but I don't want to deal with that variability internally as it has no meaning. I think an option like coerce_case or normalize_case would be a reasonable addition.

PR to follow.

[davidism]

It appears #887 already added this.

[davidism]

Never mind, this is about what value gets returned, that was about what got matched.

[sirosen]

I don't want to deal with that variability internally as it has no meaning.

Sounds like you should just be handling things in all-lowercase? #887 set things to return the normed_value -- so when case_sensitive=False, click.Choice should always be returning a lowercase string.
(Yes, this should be improved in the documentation for case_sensitive=False.)

Reading #1278, it seems this is about allowing --foo [Apple|Orange] and always returning title-case Apple or Orange even if case_sensitive=False.

Unless there's some other rationale, I don't think this is well-justified. It adds another parameter to click.Choice which is conditional on case_sensitive=False and "undoes" part of that behavior -- I think that will be confusing for most users.

I'm not saying there isn't a reason! In #569 I argued that I really have a use-case where case-sensitivity is driven by external constraints. But I'd need to know what the use-case is before agreeing that this should be part of click.

I'm reminded of this comment on the initial request for case_sensitive. If there is a clear use-case here, it's worth considering a generic parameter which changes the token_normalize_func on a per-option basis.

[altendky]

@sirosen, yes, #1278 retains the original (not necessarily title) case specified by the choices. It seems to me this may have been a better approach to begin with as opposed having case insensitivity force the introduction of a third casing (lower as opposed to original choice and user-entered). But, for backwards compatibility I introduced this as an option with default behavior remaining as it is now.

My actual use case is in https://github.com/altendky/romp where I have choices for platform and Python interpreter (and more, but specifically those). I stuck a copy of the modified choice in there which I am using for now.

https://github.com/altendky/romp/blob/742d4ac5ff0b918e33002af1f374b8ce6938367a/src/romp/_matrix.py#L11-L18

vm_images = collections.OrderedDict((
    ('Linux', 'ubuntu-16.04'),
    ('macOS', 'macOS-10.13'),
    ('Windows', 'vs2017-win2016'),
))


all_platforms = tuple(vm_images.keys())

all_platforms is used as a choice list elsewhere. I could .lower() in various places for comparison or create another mapping lower_platforms_to_display = {platform.lower(): platform for platform in all_platforms} and use that wherever I want to display the proper case. Or I could recover the case from the .lower()ed result from the Click choice. Or I could just have one casing that I work with everywhere.

As mentioned above it's not clear to me yet that the present functionality of 'returning' the lower case form is actually good over returning original or user entered. Is there a reason it is beyond (the sufficient) backwards compatibility?

Thanks for your time and feedback.

[sirosen]

Thanks for linking the original/relevant source of this issue. I was kind of guessing this might be a matter of having a source of data where case is important, then being fed into the CLI opts.

I may have to take a break from click for the moment, but will try to circle back and look again later this week -- but I can answer one question:

As mentioned above it's not clear to me yet that the present functionality of 'returning' the lower case form is actually good over returning original or user entered. Is there a reason it is beyond (the sufficient) backwards compatibility?

Maybe those who reviewed it have a different take, but as the author of #887, no, there's no reason. 😅

My use case was that I wanted --foo [apple|orange] and not to care about --foo oRanGE, but I didn't really at the time consider that the choices offered might have actually had important information encoded in the case.

We could argue that changing this behavior would be a backwards incompatible change -- demanding that it be saved for v8.0 -- or we could make the case that, as I noted that "the doc for case_sensitive should be improved", the behavior is unspecified. I would be okay with saying case_sensitive=False returns the choice value as specified, not lowercased, and calling it a fix for v7.1.
(i.e. case_sensitive=False has a bug in v7.0, in that it incorrectly returns a lowercased value instead of the matching choice value)

Realistically, most callers using case_sensitive=True are probably, like me, passing in a list of lowercase strings and would see no change.

Making the new behavior in #1278 the only behavior would almost definitely make click better to use long-term. Changing the default behavior strikes me as better than adding another option with conditional/nuanced behavior.

I want to just change it in v7.1 and field any complaints. But if we do that and someone is relying on the lowercasing and gets broken, I'm sure I'll catch some grief for taking that stance...

[altendky]

@sirosen, I can certainly make another PR to implement the 'fix' rather than the 'feature addition' in #1278. For the people banking on the forced lowercase result I guess they would get whatever misc failures in their code and the fix would be option_value = option_value.lower() (or, .casefold() perhaps if py3). An option to do that in Click itself would of course be easy but it seems it wouldn't add much value and would probably just be a generic post-proc to allow for choosing lower vs casefold vs... ?? who knows. I suppose the value in having it in Click is in reuse of defined options (I do this a good bit, x_option = click.Option('--x') then @x_option on multiple commands). Or, perhaps Click already has such a feature? I honestly am not familiar with the possible modifiers.

No rush on this on my behalf. I've got my workaround in place for now and can remove it whenever.

[sirosen]

Or, perhaps Click already has such a feature? I honestly am not familiar with the possible modifiers.

The best path on that sort of thing is to define a specialized decorator which applies your option with desired params. The callback parameter lets you do post-processing pretty cleanly:

def common_options(cmd):
    def _lower_callback(ctx, param, value):
        if value is None:
            return value
        else:
            return value.lower()

    cmd = click.option('--x', callback=_lower_callback)(cmd)
    cmd = click.option('--foo', callback=_lower_callback)(cmd)
    return cmd

---

If you're comfortable with letting this wait around a little, I'd like to hear someone else's input (e.g. @davidism if he can spare the time) on whether or not it's okay to simply change the behavior and say it was unspecified in 7.0 and will be specified in 7.1 .

[davidism]

Just to be clear, the new proposal is to make case_sensistive=True mean "match case insensitive, but return exact value", and False would be "match and return exact"?

[altendky]

@davidism, I believe so. Where 'exact' refers to 'as passed to click.Choice() in code' not 'as written on the command line'.

[davidism]

Seems fine for 7.1, along with clarifying the documentation of the option, it would be good to add an example of normalizing to lowercase if there's not one already.

[altendky]

@davdism, for the additional example, you are referring to callback=_lower_callback which would allow recovery of the existing 7.0 behavior regarding case_sensitive=False?

Note that 7.0 returns the normed value not the exact value passed to click.Choice(). So we have three values: 'exact', 'normed', and 'normed+lowered'. I haven't personally used the normalization system. Do we want to restrict this change to only correcting the result for case_sensitive=False and make it return the 'normed' value? Or to universally return the 'exact' value passed to the click.Choice() call thus also changing behavior for normalized values? The latter seems more complete and easy to clearly document.

To have the list handy, and in case I missed something, here is the documentation for click.Choice() that I found. None of them seem to clarify what will be passed to the command.

• http://click.palletsprojects.com/en/7.x/api/#click.Choice

  • click/click/types.py

    Lines 130 to 140 in baf0124

    	"""The choice type allows a value to be checked against a fixed set
    	of supported values. All of these values have to be strings.
    	You should only pass a list or tuple of choices. Other iterables
    	(like generators) may lead to surprising results.
    	See :ref:`choice-opts` for an example.
    	:param case_sensitive: Set to false to make choices case
    	insensitive. Defaults to true.
    	"""

• http://click.palletsprojects.com/en/7.x/options/#choice-opts

  • click/docs/options.rst

    Lines 311 to 344 in baf0124

    	.. _choice-opts:
    	Choice Options
    	--------------
    	Sometimes, you want to have a parameter be a choice of a list of values.
    	In that case you can use :class:`Choice` type. It can be instantiated
    	with a list of valid values.
    	Example:
    	.. click:example::
    	@click.command()
    	@click.option('--hash-type', type=click.Choice(['md5', 'sha1']))
    	def digest(hash_type):
    	click.echo(hash_type)
    	What it looks like:
    	.. click:run::
    	invoke(digest, args=['--hash-type=md5'])
    	println()
    	invoke(digest, args=['--hash-type=foo'])
    	println()
    	invoke(digest, args=['--help'])
    	.. note::
    	You should only pass the choices as list or tuple. Other iterables (like
    	generators) may lead to surprising results.
    	.. _option-prompting:

• http://click.palletsprojects.com/en/7.x/advanced/?highlight=choice#token-normalization

  • click/docs/advanced.rst

    Lines 120 to 128 in baf0124

    	Token Normalization
    	-------------------
    	.. versionadded:: 2.0
    	Starting with Click 2.0, it's possible to provide a function that is used
    	for normalizing tokens. Tokens are option names, choice values, or command
    	values. This can be used to implement case insensitive options, for
    	instance.

• http://click.palletsprojects.com/en/7.x/bashcomplete/?highlight=choice#what-it-completes

  • click/docs/bashcomplete.rst

    Lines 18 to 24 in baf0124

    	What it Completes
    	-----------------
    	Generally, the Bash completion support will complete subcommands, options
    	and any option or argument values where the type is click.Choice.
    	Subcommands and choices are always listed whereas options only if at
    	least a dash has been provided. Example::

[sirosen]

I think the best thing to do for the examples is to write a command which takes a choice like Apple|Orange|Banana and then show how it can be updated to case_insensitive=False to also accept apple, APPLE, and other spellings. Show that Apple is the string passed to the command function regardless -- and note that it is specified by the option definition, not what's passed on the command-line.

The token normalize function and use of callback on Options are both probably out of scope for this -- at least, I wouldn't add them to the examples unless lots of people show up asking questions about how they interact with this specific case.

Do we want to restrict this change to only correcting the result for case_sensitive=False and make it return the 'normed' value? Or to universally return the 'exact' value passed to the click.Choice() call thus also changing behavior for normalized values?

To clarify, you're distinguishing

• the value passed to click.Choice
  vs
• the value passed to click.Choice, passed through ctx.token_normalize_func, but without str.lower()
  ?

I think the easiest behavior to explain is that it returns the original string passed to click.Choice, not the normalized-but-not-lowercased version.

---

Also, something we haven't discussed is that case_sensitive=False will interact with this new behavior in an interesting way if multiple choices are different spellings of the same value.
With click.Choice(['apple', 'APPLE'], case_sensitive=False) -- it now will matter which one of these two choices aPPle matches.

I think it's fine for such behavior to remain undefined, but, if possible, I'd like to take the first or last matching choice in a consistent way.

[altendky]

Yes, that is the value I'm referring to.

Or maybe raise an exception on creation of a non-unique case-insensitive list? if len(set(choice.lower() for choice in choices)) < len(choices): raise... It always looked funny having the optimized direct check against the original choices at the beginning. It allowed things to match without enforcing the normalization.

[altendky]

I think these are the two open questions.

Questions:

• Should the pre-callback value always be the same as was passed to click.Choice()?
  • We already agreed that case_sensitive=False should be corrected to not modify the value
  • Presently ctx.token_normalize_func modifies the value, should that behavior be changed?
• What should be done with click.Choice(['apple', 'APPLE'], case_sensitive=False)?
  • Undefined?
  • Raise an exception?
  • Out of scope for this ticket?

[sirosen]

Presently ctx.token_normalize_func modifies the value, should that behavior be changed?

I think so. My rationale is based on looking at the diff for #887 . Prior to that, the choice value which was matched, without passing through ctx.token_normalize_func, would be returned by click.Choice.

So not applying ctx.token_normalize_func is the most similar behavior to click 6.x and below.

What should be done with click.Choice(['apple', 'APPLE'], case_sensitive=False)?

I'm fine with any of the options you put forth.
They all make sense on the grounds that "If the user isn't sure what he/she is asking for, how could Click possibly know?" :)

[altendky]

#1318 merged so the issue should be resolved.