aboutcode-org/license-expression

Licensing.parse() raises too many exceptions

Closed this issue · 4 comments

When I do:

try:
    Licensing().parse("MIT AND OR 0BSD")
except ExpressionError:
    # Handle this
    pass

The exception is not handled because a ParseError was raised instead. This isn't unexpected per se, because the docstring says as much, but I cannot see the difference between the two errors as a consumer of the library. I've rewritten my code to put except (ExpressionError, ParseError): everywhere, but it seems a little unnecessary to me.

Is it possible to make ExpressionError a subclass of ParseError? Or is it possible to consistently raise ExpressionErrors instead?

@carmenbianca good point! The ParseError comes from the boolean.py which is the underlying library that implements the core boolean logic.
There is no good reason to have this surface in here, either directly or indirectly.

Is it possible to make ExpressionError a subclass of ParseError? Or is it possible to consistently raise ExpressionErrors instead?

Any ParseError raised here should be a new subclass of ExpressionError and should eventually wrap any boolean error alright and not be a ParseError from boolean.

Could I talk you into submitting a patch? 👼

@pombredanne Sure, I'd be happy to put together a patch.

I'm a little confused by one thing, though.

Any ParseError raised here should be a new subclass of ExpressionError

I'm not sure if an exception can be subclassed on the fly. Do you mean something like

try:
    raise ParseError("oops")
except ParseError as error:
    raise ExpressionError("oops2") from error

Ah, pardon, I think I know what you mean. You want to create a ExpressionParseError that is a subclass of ExpressionError, and have that be raised instead of ParseError?

Ah, pardon, I think I know what you mean. You want to create a ExpressionParseError that is a subclass of ExpressionError, and have that be raised instead of ParseError?

@carmenbianca yes, that would be my take. But I am open to other ways.