-
Notifications
You must be signed in to change notification settings - Fork 64
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
Consistency for Error
messages
#157
Comments
Hi @LaurentAjdnik ! Sorry for not getting back to you on this sooner. I'm always in favour of having some systematic way of doing things, so I am on board with this idea. |
Hi @HGSilveri, here are some more thoughts about it. TL;DR
DiversitySometimes this information is given (when applicable), sometimes not, in error messages:
And even when we have the same info, the sentences might be built differently. StatsI might have missed a few but I counted, in the
|
One other thing: I believe that sometimes the built-in errors are abused, mostly out of laziness to not wanting to create a new error class. I would be in favour of being more strict with their definition and raising a custom Error whenever none of the built-in ones are a good fit. Would you agree? |
I agree. Custom errors can be convenient when they make sense functionally. Here, I feel like Maybe also (but not so sure) some kind of Or perhaps (even less sure) something close to Otherwise, I wouldn't specialize too much. Most errors are |
Back to the original issue of consistency for error messages. We could rewrite some of them already. Here are a few rules for
Example:
Another example:
What do you think? |
I think this is great, I have no objections. My only concern is the size of the error factory call becoming larger than the written message itself, and perhaps it being hard to decipher for someone reading the source code. Nonetheless, I'm all for this change, I look forward to seeing what you come up with! |
I totally agree. Sorry for not being more explicit. My "we could rewrite some of them already" meant "without a factory".
I'll start working on it soon, first with |
There are sooooooo many ways to write an Error message.
For instance when starting the message:
Then we have to decide whether to include the parameter value that was provided and the allowed values (see #148). And how.
Even the phrasing for allowed values can vary a lot:
Right now, along the code, we have a mix of different approaches.
Maybe we should enact rules to ensure consistency, especially since we intend to add lots of checks and hints.
As an option, we could even create an ErrorMessageFactory class.
One among very few functions:
ErrorMessageFactory.getMessage(subject, parameter, value, constraint, allowed)
ErrorMessageFactory.getMessage("The number of qubits", "n_qubits", -1, ErrorMessageFactory.EQUAL_OR_ABOVE, 1)
"The number of qubits ('n_qubits' = -1) must be 1 or above"
Of course, we would have to identify the common types of constraints and allowed values (single or interval) but it seems feasable.
At the same time, this would allow us, in test functions, to check for full Error messages with exact matches rather than partial matches or regular expressions.
Maybe I'm splitting hairs here, but I would LOVE to discuss this issue. 😁
The text was updated successfully, but these errors were encountered: