Predicate operators (& and |) raising AttributeError about cloup HOT 7 CLOSED

I decided to ship this fix and other minor fix/improvements in a patch release (v0.7.1). Of course, this doesn't include any new feature like the AllSet condition.

I'm gonna release v0.8.0 very soon.

from cloup.

Hi there! Thank you for your nice words!

It seems you caught the first bug. Thank you for reporting it! The bug affects the & and | operator of Predicate and it's very easy to fix. Curiously enough, it would not exist if I had not forgotten to write a type annotation ^.^".

I'm gonna write a fix and include it in v0.8.0, which should come out in the next days, probably tomorrow.

Is this the recommend way of setting the constraints?

Regarding what's reccomended, it depends... there are many ways to define a constraint:

• you can pass a constraint to @option_group as you did
• you can use @constraint
• you can just check your arguments inside your function and raise a click.UsageError with a manually written error when something is wrong.

If you have a complex one-time constraint that generates an unclear / non-optimal message (so you need to rephrase it with rephrased()), the advantage of using Cloup constraints over writing custom logic diminishes.

Both your constraints are formally correct but of course not equivalent. If latitude and longitude must always be provided, then you should definitely make them required.

I may help you in finding the optimal solution if you answer these questions:

• are coordinates always required?
• could you replace --latitude and --longitude with a single argument --coordinates? This would enforce the constraint that latitude and longitude must always be provided together:

cloup.option("--coordinates", metavar='LATITUDE LONGITUDE', nargs=2, type=click.FLOAT),

from cloup.

Actually my thought solution is not working so I'll have to find another way.

The options --latitude and --longitude should only be provided within the geo filtering option, because they are not needed when e.g. filtering for a station name. However when setting them to required within the option group they are always required whether or not I want to apply a geo filter.

My favorable solution is as given in the first codeblock because everything would be closed and wrapped up in one option group, arguments/options as well as constraints.

from cloup.

Yes, I suspected that making coordinates required was not what you wanted.

The problem I have with your first solution is that the generated help string and the error messages are not really clear. I mean, this is not how I would explain the constraint to another human:

exactly 3 required if --latitude is set and --longitude is set

Of course, you can use rephrased() to change the help text but changing error messages is more problematic with conditional constraints because they have two branches (and conditions cannot be rephrased now).

(By the way, I'll probably introduce an AllSet(*params) predicate so the condition description will be "--latitude and --longitude are set".)

In my opinion, you get better help text and errors if you split the constraint in two:

1. latitude and longitude go together, never alone
2. if (and only if) you provide the coordinates, then you have to provide exactly one between rank and distance.

So I would suggest one of the following solutions that will work fine in v0.8.0.

import click

import cloup
from cloup.constraints import If, IsSet, RequireExactly, accept_none, all_or_none

"""
Solution 1
"""
@cloup.command()
@cloup.option_group(
    "Latitude-Longitude rank filtering",
    cloup.option("--latitude", type=click.FLOAT),
    cloup.option("--longitude", type=click.FLOAT),
    cloup.option("--rank", type=click.INT),
    cloup.option("--distance", type=click.FLOAT),
    help="Provide the coordinates plus either --rank or --distance.",
)
@cloup.constraint(all_or_none, ['latitude', 'longitude'])
@cloup.constraint(
    If(['latitude', 'longitude'], 
        then=RequireExactly(1), 
        else_=accept_none),
    ['rank', 'distance']
)
def solution_one(**kwargs):
    pass

"""
Solution 2
--coordinates instead of --latitude and --longitude
"""
@cloup.command()
@cloup.option_group(
    "Latitude-Longitude rank filtering",
    cloup.option("--coordinates", metavar='LATITUDE LONGITUDE',
                 nargs=2, type=click.FLOAT),
    cloup.option("--rank", type=click.INT),
    cloup.option("--distance", type=click.FLOAT),
    help="Provide --coordinates plus either --rank or --distance.",
)
@cloup.constraint(
    If('coordinates', then=RequireExactly(1), else_=accept_none),
    ['rank', 'distance']
)
def solution_two(**kwargs):
    pass

In both cases, I used the option group help to provide instructions to the user. In think this is more clear than showing the costraints section (show_constraints=True) in this case.

from cloup.

Dear @janluke ,

thank you very much for this precise description! I'll be happy using v0.8.0! Thanks and have a nice weekend! :)

from cloup.

You're welcome! Have a nice weekend too!

(I'm reopening the issue. I'll close it when it's fixed.)

from cloup.

(v0.8.0 is finally out)

from cloup.